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

Analyze serialization of variadic arguments (v12 vs v13) #441

Merged
merged 6 commits into from
Apr 18, 2024

Conversation

andreibancioiu
Copy link
Contributor

@andreibancioiu andreibancioiu commented Apr 17, 2024

Follow-up of #435.

In v13, some client code that deals with Smart Contract calls might be affected by an unforeseen breaking (but minor) change.

Circumstances (affected code):

  • the client code uses SmartContract.call() to prepare transactions
  • the client code passes the abi to the SmartContract (when creating an instance)
  • the client code calls a Smart Contract endpoint that is variadic.

Solutions:

  • do not pass the abi parameter when creating the SmartContract object
  • OR adjust variadic arguments as depicted in the comments (below)
  • OR switch to using one of the new approaches (v13) of creating smart contract calls

Code that isn't affected:

  • code that uses SmartContract.methods.myFunction() to prepare a contract call
  • code that uses SmartContract.methodsExplicit.myFunction() to prepare a contract call

Under-the-hood differences:

  • in v12, SmartContract.call() only accepted TypedValue objects as args.
  • in v13, SmartContract.call() accepts a mixture of TypedValue objects and native JavaScript values / objects as args. This makes use of the NativeSerializer component, under the hood. This causes the unforeseen breaking (but minor) change.

Additional notes:

In sdk-core v13, the recommended way to create transactions for deploying, upgrading and interacting with smart contracts is through a SmartContractTransactionsFactory. The older (legacy) approaches are still available, however.

@andreibancioiu andreibancioiu changed the title Debug and adjust serializer (v13 regression) Debug serializer (v12 vs v13) Apr 18, 2024
@andreibancioiu
Copy link
Contributor Author

andreibancioiu commented Apr 18, 2024

Dealing with variadic<something>

Suppose we have the following ABI:

const abi = AbiRegistry.create({
    endpoints: [
        {
            name: "foo",
            inputs: [
                {
                    type: "variadic<u8>",
                },
            ],
        },
    ],
});

And create a SmartContract as follows:

const contract = new SmartContract({ abi, ... });

In v12, this was possible:

tx = contract.call({
    func: "foo",
    args: [new U8Value(1), new U8Value(2), new U8Value(3)],
    ...
});

In v13, the code above throws: Invalid argument: Wrong argument type for endpoint foo: typed value provided; expected variadic type, have U8Value.

Solutions

Solution A for v13 - drop abi when creating the contract object (this will completely bypass the logic of NativeSerializer, resulting in a v12-like behavior):

const contract = new SmartContract({ ... });

Solution B for v13 - adjust args as follows:

contract.call({
    func: "foo",
    args: [VariadicValue.fromItems(new U8Value(1), new U8Value(2), new U8Value(3))],
    ...
});

Solution C for v13 - adjust args as follows:

contract.call({
    func: "foo",
    args: [1, 2, 3],
    ...
});

Solution D for v13 (kindly recommended) - use the new SmartContractTransactionsFactory:

@andreibancioiu
Copy link
Contributor Author

andreibancioiu commented Apr 18, 2024

Dealing with optional<variadic<something>>

Note: having a smart contract endpoint with an optional<variadic<something>> input is extremely exotic / unusual, and not recommended. Drop the optional modifier.

Suppose we have the following ABI:

const abi = AbiRegistry.create({
    endpoints: [
        {
            name: "foo",
            inputs: [
                {
                    type: "optional<variadic<u8>>",
                },
            ],
        },
    ],
});

And create a SmartContract as follows:

const contract = new SmartContract({ abi, ... });

In v12, this was possible:

contract.call({
    func: "foo",
    args: [new U8Value(1), new U8Value(2), new U8Value(3)],
    chainID: "D",
    gasLimit: 1000000,
    caller: callerAddress,
});

In v13, the code above throws: Wrong number of arguments for endpoint foo: expected between 0 and 1 arguments, have 3.

Solutions

Solution A for v13 - drop abi when creating the contract object (this will completely bypass the logic of NativeSerializer, resulting in a v12-like behavior):

const contract = new SmartContract({ ... });

Solution B for v13 - adjust args as follows:

contract.call({
    func: "foo",
    args: [ [ new U8Value(1), new U8Value(2), new U8Value(3) ] ],
    ...
});

Solution C for v13 - adjust args as follows:

contract.call({
    func: "foo",
    args: [ [ 1, 2, 3 ] ],
    ...
});

Solution D for v13 (kindly recommended) - use the new SmartContractTransactionsFactory:

@andreibancioiu andreibancioiu marked this pull request as ready for review April 18, 2024 10:34
@@ -1,14 +1,67 @@
/* eslint-disable @typescript-eslint/no-namespace */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change in logic within the file. Only formatting.

@andreibancioiu andreibancioiu changed the title Debug serializer (v12 vs v13) Analyze serialization of variadic arguments (v12 vs v13) Apr 18, 2024
@andreibancioiu andreibancioiu merged commit 1c53e6e into feat/next Apr 18, 2024
1 check passed
@andreibancioiu andreibancioiu deleted the serializer-435 branch April 18, 2024 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants