Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON RPC converter fails to serialise classes from duplicated modules #1826

Closed
spalladino opened this issue Aug 25, 2023 · 0 comments · Fixed by #1820
Closed

JSON RPC converter fails to serialise classes from duplicated modules #1826

spalladino opened this issue Aug 25, 2023 · 0 comments · Fixed by #1820
Labels
T-bug Type: Bug. Something is broken.

Comments

@spalladino
Copy link
Collaborator

spalladino commented Aug 25, 2023

The automatic JSON RPC server and client from foundation rely on a ClassConverter object, which requires classes to be (de)serialised to be registered. Classes are indexed by the class object itself.

However, it can be the case that the same classes is loaded from different instances of the same module. We hit this error with PrivateKey: while the type registered in the aztec_rpc_client is exported from the circuits.js, an instance of the PrivateKey from the types package (which re-exports PrivateKey) is the one that gets sent.

import { AztecAddress, CompleteAddress, EthAddress, Fr, Point, PrivateKey } from '@aztec/circuits.js';

import { AztecRPC, CompleteAddress, PrivateKey } from '@aztec/types';

This would be ok if aztec.js and types loaded the same circuits.js from node_modules, but it can be the case that each package keeps its own copy of the dependency. If that happens, they are considered different modules, so each PrivateKey class is not the same. So the lookup in the converter fails.

The end result is that the PrivateKey is serialised incorrectly, so it is deseralised incorrectly on the other side, and the RPC server carries on execution with an incorrect type, which never gets validated because of #1819.

@spalladino spalladino added the T-bug Type: Bug. Something is broken. label Aug 25, 2023
@github-project-automation github-project-automation bot moved this to Todo in A3 Aug 25, 2023
spalladino added a commit that referenced this issue Aug 28, 2023
… name (#1820)

Prevents from accidentally passing an unregistered class in the
autogenerated JSON RPC client and server. Picks the converter to use for
serialisation based on constructor name if function equality match fails
(since constructor name match may fail in minimised browser bundles).

We were bit by this when the `PrivateKey` class was registered in the
client, but due to a duplicated module, the `PrivateKey` class
registered was not the same as the one passed as an argument. This
caused the object not to be properly serialised, which due to #1819 was
not picked up on the server side, and caused all sort of issues.

Fixes #1826

---------

Co-authored-by: spypsy <[email protected]>
@github-project-automation github-project-automation bot moved this from Todo to Done in A3 Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: Bug. Something is broken.
Projects
Archived in project
1 participant