Skip to content

Commit

Permalink
fix(rpc): Fixes getNodeInfo serialisation (#1991)
Browse files Browse the repository at this point in the history
When calling the RPC server across HTTP, the automagical json-rpc-server
serialisation was failing to deserialise the `NodeInfo`. The constructor
of the NodeInfo object was not exactly `Object`, so the converter did
not go recursively into each field.

This PR makes the check for "is something an object" wider, and changes
the RPC server test so it reproduces the issue without the fix. Before
this change, `getNodeInfo().rollupAddress` was equal to `{ type:
'EthAddress', data: '0x...' }`, so calling `toString()` on it resulted
in the string `[object Object]`.
  • Loading branch information
spalladino authored Sep 5, 2023
1 parent e07ea1a commit 0a29fa8
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ export const aztecRpcTestSuite = (testName: string, aztecRpcSetup: () => Promise

it('successfully gets node info', async () => {
const nodeInfo = await rpc.getNodeInfo();
expect(nodeInfo.version).toBeDefined();
expect(nodeInfo.chainId).toBeDefined();
expect(nodeInfo.rollupAddress).toBeDefined();
expect(typeof nodeInfo.version).toEqual('number');
expect(typeof nodeInfo.chainId).toEqual('number');
expect(nodeInfo.rollupAddress.toString()).toMatch(/0x[a-fA-F0-9]+/);
});

// Note: Not testing `isGlobalStateSynchronised`, `isAccountStateSynchronised` and `getSyncStatus` as these methods
Expand Down
28 changes: 17 additions & 11 deletions yarn-project/foundation/src/json-rpc/convert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,9 @@ export function convertFromJsonObj(cc: ClassConverter, obj: any): any {
if (Array.isArray(obj)) {
return obj.map((x: any) => convertFromJsonObj(cc, x));
}

// Is this a dictionary?
if (obj.constructor === Object) {
if (typeof obj === 'object') {
const newObj: any = {};
for (const key of Object.keys(obj)) {
newObj[key] = convertFromJsonObj(cc, obj[key]);
Expand All @@ -119,6 +120,7 @@ export function convertToJsonObj(cc: ClassConverter, obj: any): any {
if (!obj) {
return obj; // Primitive type
}

// Is this a Node buffer?
if (obj instanceof Buffer) {
return { type: 'Buffer', data: obj.toString('base64') };
Expand All @@ -135,22 +137,26 @@ export function convertToJsonObj(cc: ClassConverter, obj: any): any {
if (cc.isRegisteredClass(obj.constructor)) {
return cc.toJsonObj(obj);
}

// Is this an array?
if (Array.isArray(obj)) {
return obj.map((x: any) => convertToJsonObj(cc, x));
}
// Is this a dictionary?
if (obj.constructor === Object) {
const newObj: any = {};
for (const key of Object.keys(obj)) {
newObj[key] = convertToJsonObj(cc, obj[key]);

if (typeof obj === 'object') {
// Is this a dictionary?
if (isPlainObject(obj)) {
const newObj: any = {};
for (const key of Object.keys(obj)) {
newObj[key] = convertToJsonObj(cc, obj[key]);
}
return newObj;
} else {
// Throw if this is a non-primitive class that was not registered
throw new Error(`Object ${obj.constructor.name} not registered for serialisation`);
}
return newObj;
}
// Throw if this is a non-primitive class that was not registered
if (typeof obj === 'object' && !isPlainObject(obj)) {
throw new Error(`Object ${obj.constructor.name} not registered for serialisation`);
}

// Leave alone, assume JSON primitive
return obj;
}

0 comments on commit 0a29fa8

Please sign in to comment.