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

feat/1012: Improve Tx API in SDK #1033

Merged
merged 6 commits into from
Aug 28, 2024
Merged

feat/1012: Improve Tx API in SDK #1033

merged 6 commits into from
Aug 28, 2024

Conversation

jurevans
Copy link
Collaborator

@jurevans jurevans commented Aug 21, 2024

Resolves #1012

This PR refactors the response from the various build_ functions, including build_batch, to be able to provide via the SDK package a deserialized object that can be inspected and destructured. I think in the future, the @heliax/namada-sdk should never return a reference to a Rust struct, as it's clunky and breaks TS convention to use. This PR returns an normal TS type where values can be inspected and destructured.

NOTES

Instead of returning an instance of a Rust struct BuiltTx to the client, return a deserialized tx value:

import { WrapperTxProps } from "@namada/types";

// We'll use borsh schemas instead of type defs, but essentially what the user
// will see is this:

type SigningData = {
  publicKeys: string[];
  threshold: number;
  feePayer: string;
  accountPublicKeysMap?: Uint8Array;
  owner?: string;
}

type Tx = {
  // tx hash
  hash: string;
  // tx bytes
  bytes: Uint8Array;
  // args used to create args::Tx in namada-sdk
  args: WrapperTxProps;
  // deserialized signing data 
  signingData: SigningData[];
}

This makes the result of any build function something that can be inspected or destructured.

Testing

  • Simply build this branch and test existing Tx functionality

Example Tx

{
    "args": {
        "token": "tnam1q87wtaqqtlwkw927gaff34hgda36huk0kgry692a",
        "feeAmount": "0.000001",
        "gasLimit": "300000",
        "chainId": "internal-devnet-43.35fd77a4447",
        "publicKey": "tpknam1qr4w8d26738k7zmhxhsp9987puunenrjt4kxp5f5k8wlgey3zzuyqg9tgqh",
        "memo": ""
    },
    "hash": "0CEBA68C9BB9391B5481C46069B32EFE45758E77C5114D676455F471A711A2F5",
    "bytes": {
        "type": "Buffer",
        "data": [
          ...
        ]
    },
    "signingData": [
        {
            "owner": "tnam1qqql82l8d8xcgu8q598gl29vecmelc9v6vfyzwn3",
            "publicKeys": [
                "tpknam1qr4w8d26738k7zmhxhsp9987puunenrjt4kxp5f5k8wlgey3zzuyqg9tgqh"
            ],
            "threshold": 1,
            "accountPublicKeysMap": {
                "type": "Buffer",
                "data": [
                   ...
                ]
            },
            "feePayer": "tpknam1qr4w8d26738k7zmhxhsp9987puunenrjt4kxp5f5k8wlgey3zzuyqg9tgqh"
        }
    ]
}

@jurevans jurevans self-assigned this Aug 21, 2024
@jurevans jurevans added this to the PoS, Governance milestone Aug 21, 2024
@jurevans jurevans force-pushed the feat/1012-sdk-api-updates branch 3 times, most recently from 96c35fd to 1d8ede5 Compare August 23, 2024 13:44
@jurevans jurevans marked this pull request as ready for review August 23, 2024 14:47
@jurevans jurevans changed the title WIP: feat/1012: Improve Tx API in SDK feat/1012: Improve Tx API in SDK Aug 23, 2024
@jurevans jurevans force-pushed the feat/1012-sdk-api-updates branch 3 times, most recently from 0a1516d to 42dcbcf Compare August 26, 2024 13:48
Copy link
Contributor

@euharrison euharrison left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@@ -22,6 +22,7 @@ export const LedgerConfirmation = (): JSX.Element => {
<ViewKeys
publicKeyAddress={account.publicKey}
transparentAccountAddress={account.address}
trimCharacters={35}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Missed this in a previous PR

Copy link
Collaborator

@mateuszjasiuk mateuszjasiuk left a comment

Choose a reason for hiding this comment

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

Tested everything except withdraw as I have to wait 8 hours xD LGTM!

packages/shared/lib/src/sdk/mod.rs Outdated Show resolved Hide resolved
@jurevans jurevans merged commit 862de2a into main Aug 28, 2024
8 checks passed
@jurevans jurevans deleted the feat/1012-sdk-api-updates branch August 28, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SDK: Improve result of build functions
3 participants