solana-labs / solana-web3.js

Solana JavaScript SDK
https://solana-labs.github.io/solana-web3.js
MIT License
2.18k stars 865 forks source link

partialSign will compile and serialize the message of transaction and so that the signData will be changed. #3244

Closed leverwwz closed 1 month ago

leverwwz commented 1 month ago

Overview

Steps to reproduce

here is the source code of solana-web3.js src/transaction/legacy.ts:698

step 1, generate a partial signed transaction(base58-string) const partialSignedTx = '4aRori5FxxdmCXksyvKbkYKphatTz2cVzeWU7YfxDqxUsMaagAsqRBDwX8dgNLaeKv6MV8a14idJqPKQKowcZWcU1JtXFDByV528CHuGXPU2iCzqABtbMCHtC5hUZicnGeLqYdMFp6qRdH7HeqnB1EH8Nh6JbUhL1YswJ2Ezv5SULLfkJUnYRxwXtqKDka4jULMqmG3ZnKMdLkpQbiCFuGgExzTq9jB3hbXwzuhSx2PMc1gzgvN93716Vvv6pwv1h8hebr6u48Me3ckpgM71JF9WD2EsPCyqX4nybJoNdKJDH6AP3PxPDvf9fmSNYsbS8rHeJZVVieFfX4DVirqQ7CvjczsUA38xMCvovLficnJGzMCVhEGzYZaoAiLX9zN6uXgRFSjBkYTEiPN1GkRcyq3padgKe6hbNJZQWNPV2FT2QUxtTfCWTqBoT4zcPCshcDhKD6BbnGGy69hFctsYNpuT67Y8PygR6Ry2NZqNw3DHfMGiqULKk7R2fJpw84gMT5vqeRA832tbpDAFmPb9MH4ZJ1sjiPicKMMCEi4DN3RyaXjJXbZudcS3x6nYTGqZ9LdbpQ23Eh2TLEZ3KuPoC9V3pWMquRpppw6XmsFCM7MA13qY2hBrjr5xv2Uf3rh5KojbYJnz4KqYY4zpywCK9M2j5P6V8g22RoFYGBKLk';

step 2, const tx = Transaction.from(bs58.decode(partialSignedTx)); tx.partialSign(someSigner);

step3, tx.serialize()

you will find the problem.

Description of bug

we get the signData first, and then sign it and get the signature. push the signature and its public key into the ts.signatures, this will work well.

please help to recheck the code of src/transaction/legacy.ts:374

/**

i think here is a bug.

please think about this, thank you.

steveluscher commented 1 month ago

You're absolutely correct! There's no specification on the order in which addresses etc. should be serialized in a transaction, so re-serializing it might sort them in a different order, invalidating existing signatures.

We fixed this in web3.js 2.0 by creating a Transaction type that is just the message bytes and a map of signatures and creating a partial sign function that operates over that without modifying the message bytes.

Read more here: https://github.com/solana-labs/solana-web3.js/releases/tag/v2.0.0-rc.1

leverwwz commented 1 month ago

thank u

You're absolutely correct! There's no specification on the order in which addresses etc. should be serialized in a transaction, so re-serializing it might sort them in a different order, invalidating existing signatures.

We fixed this in web3.js 2.0 by creating a Transaction type that is just the message bytes and a map of signatures and creating a partial sign function that operates over that without modifying the message bytes.

Read more here: https://github.com/solana-labs/solana-web3.js/releases/tag/v2.0.0-rc.1

thank u so much

github-actions[bot] commented 3 weeks ago

Because there has been no activity on this issue for 7 days since it was closed, it has been automatically locked. Please open a new issue if it requires a follow up.