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

api: Use serialized signature for execute_transaction endpoint #7185

Merged
merged 1 commit into from Jan 10, 2023

Conversation

joyqvq
Copy link
Contributor

@joyqvq joyqvq commented Jan 6, 2023

What

Make sui_executeTransaction API the same as sui_executeTransactionSerializedSig.

This sui_executeTransaction now requires the client to be responsible to serialize flag || signature || pubkey in one field and submit in the signature field. Instead of sending three separate fields previously (scheme, pubkey, signature).

Why

This simplifies future use cases for multisig and multi agent execute transaction.

How to use

In Rust:

let serialized_sig = crypto::Signature::from_bytes(&[&*flag, &*sig_bytes, &*pub_key].concat()).unwrap();

In CLI: See usage of sui keytool sign.

target/debug/sui keytool sign --address 0xb59ce11ef3ad15b6c247dda9890dce1b781f99df --data $DATA_TO_SIGN

Intent message to sign: AAAAAAP986VtisOQSZxhH9M4A24xOaDppQDue7TlY/36sS2HyepBJa2PjB3RkxSAjb+7Pv1voJYk/RjX9AlYZ5+hAgAAAAAAAAAgghpx3ucYetjUIHnaFCho6iaUXnt4hczdAeLlgIw0GqsBAAAAAAAAAOgDAAAAAAAA
Signer address: 0xb59ce11ef3ad15b6c247dda9890dce1b781f99df
Serialized signature (`flag || sig || pk` in Base64): $SERIALIZED_SIG

In Typescript:

      // Serialize signature field as: `flag || sig || pk`
        const serialized_sig = new Uint8Array(
          1 + signature.getLength() + pubkey.toBytes().length
        );
        serialized_sig.set([SIGNATURE_SCHEME_TO_FLAG[signatureScheme]]);
        serialized_sig.set(signature.getData(), 1);
        serialized_sig.set(pubkey.toBytes(), 1 + signature.getLength());

How to Prepare Ahead of Time

You can switch to sui_executeTransactionSerializedSig (https://docs.sui.io/sui-jsonrpc#sui_executeTransactionSerializedSig) now to test the serialization in the current deployed Sui. After this commit is release, you can switch back to sui_executeTransaction anytime with the same param.

Also fixed some ergonomics and bugs of keytool and offline signing docs.

@vercel
Copy link

vercel bot commented Jan 6, 2023

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

Name Status Preview Comments Updated
wallet-adapter ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 10, 2023 at 9:46AM (UTC)
1 Ignored Deployment
Name Status Preview Comments Updated
explorer-storybook ⬜️ Ignored (Inspect) Jan 10, 2023 at 9:46AM (UTC)

@github-actions github-actions bot added the Type: Documentation Improvements or additions to documentation label Jan 6, 2023
@joyqvq joyqvq requested a review from longbowlu January 6, 2023 07:39
@longbowlu
Copy link
Contributor

should we rename sui_executeTransactionSerializedSig to sui_executeTransaction instead?

@joyqvq
Copy link
Contributor Author

joyqvq commented Jan 9, 2023

should we rename sui_executeTransactionSerializedSig to sui_executeTransaction instead?

renaming API is dangerous here since we have to ensure a deployment of wallet does not happen before sui deploy, because otherwise the current version of wallet calls into the sui_executeTransactionSerializedSig and the endpoint is gone

@longbowlu
Copy link
Contributor

should we rename sui_executeTransactionSerializedSig to sui_executeTransaction instead?

renaming API is dangerous here since we have to ensure a deployment of wallet does not happen before sui deploy, because otherwise the current version of wallet calls into the sui_executeTransactionSerializedSig and the endpoint is gone

So I was thinking we change sui_executeTransaction to do the exactly same thing as sui_executeTransactionSerializedSig and deprecate sui_executeTransactionSerializedSig gradually, because its name is too long.

@joyqvq joyqvq changed the title api: deprecate execute_transaction endpoint api: Use serialized signature for execute_transaction endpoint Jan 10, 2023
@joyqvq
Copy link
Contributor Author

joyqvq commented Jan 10, 2023

should we rename sui_executeTransactionSerializedSig to sui_executeTransaction instead?

renaming API is dangerous here since we have to ensure a deployment of wallet does not happen before sui deploy, because otherwise the current version of wallet calls into the sui_executeTransactionSerializedSig and the endpoint is gone

So I was thinking we change sui_executeTransaction to do the exactly same thing as sui_executeTransactionSerializedSig and deprecate sui_executeTransactionSerializedSig gradually, because its name is too long.

i see what you mean, this works! revised. i will have wallet still points to sui_executeTransactionSerializedSig here and once this change is deployed with sui, we can make wallet to call sui_executeTransaction

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Type: Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants