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

[TypeScript SDK] Allow people to pass Pure args directly #7765

Merged
merged 13 commits into from
Feb 1, 2023

Conversation

xydas97
Copy link
Contributor

@xydas97 xydas97 commented Jan 27, 2023

Added PureArg type in sui-bcs.ts
If Move Call argument is of type PureArg, it is returned without further serialization in call-arg-serializer.ts

closes #7569
closes #7539

@vercel
Copy link

vercel bot commented Jan 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
explorer ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 31, 2023 at 4:47PM (UTC)
frenemies ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 31, 2023 at 4:47PM (UTC)
wallet-adapter ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 31, 2023 at 4:47PM (UTC)
1 Ignored Deployment
Name Status Preview Comments Updated
explorer-storybook ⬜️ Ignored (Inspect) Jan 31, 2023 at 4:47PM (UTC)

@vercel vercel bot temporarily deployed to Preview – wallet-adapter January 27, 2023 12:56 Inactive
@vercel vercel bot temporarily deployed to Preview – frenemies January 27, 2023 12:56 Inactive
@xydas97 xydas97 marked this pull request as ready for review January 27, 2023 12:57
@xydas97 xydas97 requested review from 666lcz and patrickkuo January 27, 2023 12:57
@@ -57,7 +57,7 @@ export type ObjectOwner = Infer<typeof ObjectOwner>;

// TODO: Figure out if we actually should have validaton on this:
export const SuiJsonValue = unknown();
export type SuiJsonValue = boolean | number | string | Array<SuiJsonValue>;
export type SuiJsonValue = boolean | number | string | PureArg | Array<SuiJsonValue>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

MoveCallTransaction interface has this property arguments: SuiJsonValue[] so I added PureArg as SuiJsonValue but I am not sure if this is an acceptable way to do it

Copy link
Contributor

Choose a reason for hiding this comment

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

PureArg should not be part of SuiJsonValue as it does not work with the RpcTxnBuilder. I see two ways of fixing this:

  1. change arguments: SuiJsonValue[] to arguments: (SuiJsonValue | PureArg)[]. To make this compatible with the RpcTxnDataSerializer, we either throw an error if PureArg is passed as an argument in RpcTxnDataSerializer.ts

  2. Or we can update the rust code

    rpc_arguments: Vec<SuiJsonValue>,
    to include PureArg in SuiJsonValue

cc @oxade

/**
* A pure argument.
*/
export type PureArg = { Pure: ArrayLike<number> };
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: reuse in line 167

Comment on lines 2 to 5
"@mysten/sui.js": minor
---

Pure args can be passed directly
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"@mysten/sui.js": minor
---
Pure args can be passed directly
"@mysten/sui.js": patch
---
Allow passing Pure args directly in Move call

We are following the semver semantics for Alpha and beta (0.x.y) : https://github.com/dbrock/semver-howto#alpha-and-beta-0xy, incrementing y for every backward-compatible change, and x for every incompatible change. So this should be a patch change since it's backward compatible

@@ -57,7 +57,7 @@ export type ObjectOwner = Infer<typeof ObjectOwner>;

// TODO: Figure out if we actually should have validaton on this:
export const SuiJsonValue = unknown();
export type SuiJsonValue = boolean | number | string | Array<SuiJsonValue>;
export type SuiJsonValue = boolean | number | string | PureArg | Array<SuiJsonValue>;
Copy link
Contributor

Choose a reason for hiding this comment

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

PureArg should not be part of SuiJsonValue as it does not work with the RpcTxnBuilder. I see two ways of fixing this:

  1. change arguments: SuiJsonValue[] to arguments: (SuiJsonValue | PureArg)[]. To make this compatible with the RpcTxnDataSerializer, we either throw an error if PureArg is passed as an argument in RpcTxnDataSerializer.ts

  2. Or we can update the rust code

    rpc_arguments: Vec<SuiJsonValue>,
    to include PureArg in SuiJsonValue

cc @oxade

@vercel vercel bot temporarily deployed to Preview – wallet-adapter January 31, 2023 16:46 Inactive
@vercel vercel bot temporarily deployed to Preview – frenemies January 31, 2023 16:47 Inactive
@xydas97 xydas97 merged commit 0341586 into main Feb 1, 2023
@xydas97 xydas97 deleted the TS-SDK-PureArgs-Andreas branch February 1, 2023 12:02
williampsmith pushed a commit that referenced this pull request Feb 3, 2023
Added PureArg type in sui-bcs.ts
If Move Call argument is of type PureArg, it is returned without further
serialization in call-arg-serializer.ts

closes #7569
closes #7539

---------

Co-authored-by: Chris Li <[email protected]>
@PaulFidika
Copy link
Contributor

curl --location --request POST $SUI_RPC_HOST \
--header 'Content-Type: application/json' \
--data-raw '{
    "jsonrpc": "2.0",
    "id": 1,
    "method": "sui_batchTransaction",
    "params": [
        "0xbb81965d327c51d42d1081e5d81909652f05a675",
        [
            {
                "moveCallRequestParams": {
                    "packageObjectId": "0xa7b5d34fd01c30201076521b6feb2b4b5e0c7532",
                    "module": "metadata",
                    "function": "view_all",
                    "typeArguments": [],
                    "arguments": ["0x3fcd6d18b440acc67d2a40afc40357b29681fb51", "0x6bd0af67e5634dca308f4674b9e770bb2b1f0bc6"]
                }
            }
        ],
        "0x6b816d23fc88eca23c8923b9a8460c2481891c51",
        2000,
        "DevInspect"
    ]
}'

How do I construct a devInspect transaction using a UID? Right now this is giving the error:

{"jsonrpc":"2.0","error":{"code":-32000,"message":"ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: EntryArgumentError(EntryArgumentError { argument_idx: 0, kind: TypeMismatch }), source: Some(\"Expected argument of type 0x2::object::UID, but found type 0x311fba79f29e8ce8c5fb755a00e322aa813e456a::outlaw_sky::Outlaw\") } }"},"id":1}

How do I make the first argument to be of type UID inside of the object, rather than the object itself?

@PaulFidika
Copy link
Contributor

PaulFidika commented Mar 16, 2023

Is this part of production? I'm using Typescript SDK v0.29 and I'm still getting a Tyepscript error about how Uint8Array's aren't part of SuiJSON or SuiJSON[ ] types and I can't use it as a function argument. When I do a @ts-ignore and do it anyway, I get a weird error like this:

"arguments": [
      "0x3a0d94db8142cfd333aab0ee1f38b0c48eb20034",
      [
        {
          "0": 9,
          "1": 68,
          "2": 117,
          "3": 115,
          "4": 116,
          "5": 32,
          "6": 76,
          "7": 97,
          "8": 98,
          "9": 115
        },
        {
          "0": 25,
          "1": 104,
          "2": 116,
          "3": 116,
          "4": 112,
          "5": 115,
          "6": 58,
          "7": 47,
          "8": 47,
          "9": 119,
          "10": 119,
          "11": 119,
          "12": 46,
          "13": 100,
          "14": 117,
          "15": 115,
          "16": 116,
          "17": 108,
          "18": 97,
          "19": 98,
          "20": 115,
          "21": 46,
          "22": 99,
          "23": 111,
          "24": 109,
          "25": 47
        }
      ],
      "0x32f4b259c385896f9c7b6e80116468aebc4d0975"
    ],
    "gasBudget": 2000
  }
}: Error: RPC Error: Could not serialize argument of type Vector(Vector(U8)) at 1 into vector<vector<u8>>. Got error: Unexpected arg {"0":9,"1":68,"2":117,"3":115,"4":116,"5":32,"6":76,"7":97,"8":98,"9":115} for expected type vector<u8>

Not sure why it turns my Uint8Array into this weird object...

What's really weird about this too is that bcs.ser (the official BCS serializer) returns Unit8Arrays, so you need to turn bcs's output into a number[ ][ ] instead to get the Sui Typescript SDK to accept it as input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants