diff --git a/web3.js/module.d.ts b/web3.js/module.d.ts index 2c6f5ed239..ac58b72ae1 100644 --- a/web3.js/module.d.ts +++ b/web3.js/module.d.ts @@ -653,7 +653,7 @@ declare module '@solana/web3.js' { instructions: Array; recentBlockhash?: Blockhash; nonceInfo?: NonceInformation; - feePayer: PublicKey | null; + feePayer?: PublicKey; constructor(opts?: TransactionCtorFields); static from(buffer: Buffer | Uint8Array | Array): Transaction; diff --git a/web3.js/module.flow.js b/web3.js/module.flow.js index e23b2c0ee7..870eafa669 100644 --- a/web3.js/module.flow.js +++ b/web3.js/module.flow.js @@ -657,7 +657,7 @@ declare module '@solana/web3.js' { instructions: Array; recentBlockhash: ?Blockhash; nonceInfo: ?NonceInformation; - feePayer: PublicKey | null; + feePayer: ?PublicKey; constructor(opts?: TransactionCtorFields): Transaction; static from(buffer: Buffer | Uint8Array | Array): Transaction; diff --git a/web3.js/src/message.js b/web3.js/src/message.js index 84857c0cb6..444381c1ec 100644 --- a/web3.js/src/message.js +++ b/web3.js/src/message.js @@ -83,16 +83,6 @@ export class Message { ); } - findSignerIndex(signer: PublicKey): number { - const index = this.accountKeys.findIndex(accountKey => { - return accountKey.equals(signer); - }); - if (index < 0) { - throw new Error(`unknown signer: ${signer.toString()}`); - } - return index; - } - serialize(): Buffer { const numKeys = this.accountKeys.length; diff --git a/web3.js/src/transaction.js b/web3.js/src/transaction.js index 3d53946a29..4e5a371a4e 100644 --- a/web3.js/src/transaction.js +++ b/web3.js/src/transaction.js @@ -112,12 +112,14 @@ type SignaturePubkeyPair = {| * * @typedef {Object} TransactionCtorFields * @property {?Blockhash} recentBlockhash A recent blockhash + * @property {?PublicKey} feePayer The transaction fee payer * @property {?Array} signatures One or more signatures * */ type TransactionCtorFields = {| recentBlockhash?: Blockhash | null, nonceInfo?: NonceInformation | null, + feePayer?: PublicKey | null, signatures?: Array, |}; @@ -154,14 +156,9 @@ export class Transaction { } /** - * The transaction fee payer (first signer) + * The transaction fee payer */ - get feePayer(): PublicKey | null { - if (this.signatures.length > 0) { - return this.signatures[0].publicKey; - } - return null; - } + feePayer: ?PublicKey; /** * The instructions to atomically execute @@ -169,15 +166,15 @@ export class Transaction { instructions: Array = []; /** - * A recent transaction id. Must be populated by the caller + * A recent transaction id. Must be populated by the caller */ - recentBlockhash: Blockhash | null; + recentBlockhash: ?Blockhash; /** * Optional Nonce information. If populated, transaction will use a durable * Nonce hash instead of a recentBlockhash. Must be populated by the caller */ - nonceInfo: NonceInformation | null; + nonceInfo: ?NonceInformation; /** * Construct an empty Transaction @@ -228,8 +225,14 @@ export class Transaction { throw new Error('No instructions provided'); } - if (this.feePayer === null) { - throw new Error('Transaction feePayer required'); + let feePayer: PublicKey; + if (this.feePayer) { + feePayer = this.feePayer; + } else if (this.signatures.length > 0 && this.signatures[0].publicKey) { + // Use implicit fee payer + feePayer = this.signatures[0].publicKey; + } else { + throw new Error('Transaction fee payer required'); } const programIds: string[] = []; @@ -277,31 +280,41 @@ export class Transaction { } }); - // Move payer to the front and disallow unknown signers - this.signatures.forEach((signature, signatureIndex) => { - const isPayer = signatureIndex === 0; + // Move fee payer to the front + const feePayerIndex = uniqueMetas.findIndex(x => { + return x.pubkey.equals(feePayer); + }); + if (feePayerIndex > -1) { + const [payerMeta] = uniqueMetas.splice(feePayerIndex, 1); + payerMeta.isSigner = true; + payerMeta.isWritable = true; + uniqueMetas.unshift(payerMeta); + } else { + uniqueMetas.unshift({ + pubkey: feePayer, + isSigner: true, + isWritable: true, + }); + } + + // Disallow unknown signers + for (const signature of this.signatures) { const uniqueIndex = uniqueMetas.findIndex(x => { return x.pubkey.equals(signature.publicKey); }); if (uniqueIndex > -1) { - if (isPayer) { - const [payerMeta] = uniqueMetas.splice(uniqueIndex, 1); - payerMeta.isSigner = true; - payerMeta.isWritable = true; - uniqueMetas.unshift(payerMeta); - } else { + if (!uniqueMetas[uniqueIndex].isSigner) { uniqueMetas[uniqueIndex].isSigner = true; + console.warn( + 'Transaction references a signature that is unnecessary, ' + + 'only the fee payer and instruction signer accounts should sign a transaction. ' + + 'This behavior is deprecated and will throw an error in the next major version release.', + ); } - } else if (isPayer) { - uniqueMetas.unshift({ - pubkey: signature.publicKey, - isSigner: true, - isWritable: true, - }); } else { throw new Error(`unknown signer: ${signature.publicKey.toString()}`); } - }); + } let numRequiredSignatures = 0; let numReadonlySignedAccounts = 0; @@ -325,18 +338,14 @@ export class Transaction { } }); - if (numRequiredSignatures !== this.signatures.length) { - throw new Error('missing signer(s)'); - } - const accountKeys = signedKeys.concat(unsignedKeys); const instructions: CompiledInstruction[] = this.instructions.map( instruction => { const {data, programId} = instruction; return { programIdIndex: accountKeys.indexOf(programId.toString()), - accounts: instruction.keys.map(keyObj => - accountKeys.indexOf(keyObj.pubkey.toString()), + accounts: instruction.keys.map(meta => + accountKeys.indexOf(meta.pubkey.toString()), ), data: bs58.encode(data), }; @@ -360,11 +369,37 @@ export class Transaction { }); } + /** + * @private + */ + _compile(): Message { + const message = this.compileMessage(); + const signedKeys = message.accountKeys.slice( + 0, + message.header.numRequiredSignatures, + ); + + if (this.signatures.length === signedKeys.length) { + const valid = this.signatures.every((pair, index) => { + return signedKeys[index].equals(pair.publicKey); + }); + + if (valid) return message; + } + + this.signatures = signedKeys.map(publicKey => ({ + signature: null, + publicKey, + })); + + return message; + } + /** * Get a buffer of the Transaction data that need to be covered by signatures */ serializeMessage(): Buffer { - return this.compileMessage().serialize(); + return this._compile().serialize(); } /** @@ -372,6 +407,10 @@ export class Transaction { * The first signer will be used as the transaction fee payer account. * * Signatures can be added with either `partialSign` or `addSignature` + * + * @deprecated Deprecated since v0.84.0. Only the fee payer needs to be + * specified and it can be set in the Transaction constructor or with the + * `feePayer` property. */ setSigners(...signers: Array) { if (signers.length === 0) { @@ -395,8 +434,10 @@ export class Transaction { /** * Sign the Transaction with the specified accounts. Multiple signatures may * be applied to a Transaction. The first signature is considered "primary" - * and is used when testing for Transaction confirmation. The first signer - * will be used as the transaction fee payer account. + * and is used identify and confirm transactions. + * + * If the Transaction `feePayer` is not set, the first signer will be used + * as the transaction fee payer account. * * Transaction fields should not be modified after the first call to `sign`, * as doing so may invalidate the signature and cause the Transaction to be @@ -409,28 +450,33 @@ export class Transaction { throw new Error('No signers'); } + // Dedupe signers const seen = new Set(); - this.signatures = signers - .filter(signer => { - const key = signer.publicKey.toString(); - if (seen.has(key)) { - return false; - } else { - seen.add(key); - return true; - } - }) - .map(signer => ({ - signature: null, - publicKey: signer.publicKey, - })); + const uniqueSigners = []; + for (const signer of signers) { + const key = signer.publicKey.toString(); + if (seen.has(key)) { + continue; + } else { + seen.add(key); + uniqueSigners.push(signer); + } + } - this.partialSign(...signers); + this.signatures = uniqueSigners.map(signer => ({ + signature: null, + publicKey: signer.publicKey, + })); + + const message = this._compile(); + this._partialSign(message, ...uniqueSigners); + this._verifySignatures(message.serialize(), true); } /** * Partially sign a transaction with the specified accounts. All accounts must - * correspond to a public key that was previously provided to `setSigners`. + * correspond to either the fee payer or a signer account in the transaction + * instructions. * * All the caveats from the `sign` method apply to `partialSign` */ @@ -439,25 +485,48 @@ export class Transaction { throw new Error('No signers'); } - const message = this.compileMessage(); - this.signatures.sort(function (x, y) { - const xIndex = message.findSignerIndex(x.publicKey); - const yIndex = message.findSignerIndex(y.publicKey); - return xIndex < yIndex ? -1 : 1; - }); + // Dedupe signers + const seen = new Set(); + const uniqueSigners = []; + for (const signer of signers) { + const key = signer.publicKey.toString(); + if (seen.has(key)) { + continue; + } else { + seen.add(key); + uniqueSigners.push(signer); + } + } + const message = this._compile(); + this._partialSign(message, ...uniqueSigners); + } + + /** + * @private + */ + _partialSign(message: Message, ...signers: Array) { const signData = message.serialize(); signers.forEach(signer => { const signature = nacl.sign.detached(signData, signer.secretKey); - this.addSignature(signer.publicKey, signature); + this._addSignature(signer.publicKey, signature); }); } /** * Add an externally created signature to a transaction. The public key - * must correspond to a public key that was previously provided to `setSigners`. + * must correspond to either the fee payer or a signer account in the transaction + * instructions. */ addSignature(pubkey: PublicKey, signature: Buffer) { + this._compile(); // Ensure signatures array is populated + this._addSignature(pubkey, signature); + } + + /** + * @private + */ + _addSignature(pubkey: PublicKey, signature: Buffer) { invariant(signature.length === 64); const index = this.signatures.findIndex(sigpair => @@ -600,6 +669,9 @@ export class Transaction { static populate(message: Message, signatures: Array): Transaction { const transaction = new Transaction(); transaction.recentBlockhash = message.recentBlockhash; + if (message.header.numRequiredSignatures > 0) { + transaction.feePayer = message.accountKeys[0]; + } signatures.forEach((signature, index) => { const sigPubkeyPair = { signature: diff --git a/web3.js/test/bpf-loader.test.js b/web3.js/test/bpf-loader.test.js index 9bc1f47c67..76090c04ab 100644 --- a/web3.js/test/bpf-loader.test.js +++ b/web3.js/test/bpf-loader.test.js @@ -176,7 +176,7 @@ describe('load BPF Rust program', () => { ); }); - test('simulate transaction without signature verification', async () => { + test('deprecated - simulate transaction without signature verification', async () => { const simulatedTransaction = new Transaction().add({ keys: [ {pubkey: payerAccount.publicKey, isSigner: true, isWritable: true}, @@ -202,6 +202,33 @@ describe('load BPF Rust program', () => { ); }); + test('simulate transaction without signature verification', async () => { + const simulatedTransaction = new Transaction({ + feePayer: payerAccount.publicKey, + }).add({ + keys: [ + {pubkey: payerAccount.publicKey, isSigner: true, isWritable: true}, + ], + programId: program.publicKey, + }); + + const {err, logs} = ( + await connection.simulateTransaction(simulatedTransaction) + ).value; + expect(err).toBeNull(); + + if (logs === null) { + expect(logs).not.toBeNull(); + return; + } + + expect(logs.length).toBeGreaterThanOrEqual(2); + expect(logs[0]).toEqual(`Call BPF program ${program.publicKey.toBase58()}`); + expect(logs[logs.length - 1]).toEqual( + `BPF program ${program.publicKey.toBase58()} success`, + ); + }); + test('simulate transaction with bad programId', async () => { const simulatedTransaction = new Transaction().add({ keys: [ diff --git a/web3.js/test/connection.test.js b/web3.js/test/connection.test.js index 0ac6a9f4f0..7b54a276e1 100644 --- a/web3.js/test/connection.test.js +++ b/web3.js/test/connection.test.js @@ -198,8 +198,8 @@ test('get program accounts', async () => { }, ]); - if (transaction.recentBlockhash === null) { - expect(transaction.recentBlockhash).not.toBeNull(); + if (!transaction.recentBlockhash) { + expect(transaction.recentBlockhash).toBeTruthy(); return; } diff --git a/web3.js/test/transaction.test.js b/web3.js/test/transaction.test.js index 11edc4934a..8281089d24 100644 --- a/web3.js/test/transaction.test.js +++ b/web3.js/test/transaction.test.js @@ -86,19 +86,22 @@ describe('compileMessage', () => { expect(() => { transaction.compileMessage(); - }).toThrow('Transaction feePayer required'); - - transaction.setSigners(payer.publicKey); - - expect(() => { - transaction.compileMessage(); - }).toThrow('missing signer'); + }).toThrow('Transaction fee payer required'); transaction.setSigners(payer.publicKey, new Account().publicKey); expect(() => { transaction.compileMessage(); }).toThrow('unknown signer'); + + // Expect compile to succeed with implicit fee payer from signers + transaction.setSigners(payer.publicKey); + transaction.compileMessage(); + + // Expect compile to succeed with fee payer and no signers + transaction.signatures = []; + transaction.feePayer = payer.publicKey; + transaction.compileMessage(); }); test('payer is writable', () => { @@ -260,6 +263,7 @@ test('transfer signatures', () => { const newTransaction = new Transaction({ recentBlockhash: orgTransaction.recentBlockhash, + feePayer: orgTransaction.feePayer, signatures: orgTransaction.signatures, }).add(transfer1, transfer2); @@ -354,7 +358,10 @@ test('parse wire format and serialize', () => { toPubkey: recipient, lamports: 49, }); - const expectedTransaction = new Transaction({recentBlockhash}).add(transfer); + const expectedTransaction = new Transaction({ + recentBlockhash, + feePayer: sender.publicKey, + }).add(transfer); expectedTransaction.sign(sender); const wireTransaction = Buffer.from( @@ -423,21 +430,38 @@ test('serialize unsigned transaction', () => { expect(expectedTransaction.signatures.length).toBe(0); expect(() => { expectedTransaction.serialize(); - }).toThrow(Error); + }).toThrow('Transaction fee payer required'); expect(() => { expectedTransaction.serialize({verifySignatures: false}); - }).toThrow(Error); + }).toThrow('Transaction fee payer required'); expect(() => { expectedTransaction.serializeMessage(); - }).toThrow('Transaction feePayer required'); + }).toThrow('Transaction fee payer required'); + expectedTransaction.feePayer = sender.publicKey; + + // Transactions with missing signatures will fail sigverify. + expect(() => { + expectedTransaction.serialize(); + }).toThrow('Signature verification failed'); + + // Serializing without signatures is allowed if sigverify disabled. + expectedTransaction.serialize({verifySignatures: false}); + + // Serializing the message is allowed when signature array has null signatures + expectedTransaction.serializeMessage(); + + expectedTransaction.feePayer = null; expectedTransaction.setSigners(sender.publicKey); expect(expectedTransaction.signatures.length).toBe(1); - // Signature array populated with null signatures fails. + // Transactions with missing signatures will fail sigverify. expect(() => { expectedTransaction.serialize(); - }).toThrow(Error); + }).toThrow('Signature verification failed'); + + // Serializing without signatures is allowed if sigverify disabled. + expectedTransaction.serialize({verifySignatures: false}); // Serializing the message is allowed when signature array has null signatures expectedTransaction.serializeMessage(); @@ -468,7 +492,7 @@ test('serialize unsigned transaction', () => { expect(expectedTransaction.signatures.length).toBe(1); }); -test('externally signed stake delegate', () => { +test('deprecated - externally signed stake delegate', () => { const from_keypair = nacl.sign.keyPair.fromSeed( Uint8Array.from(Array(32).fill(1)), ); @@ -489,3 +513,25 @@ test('externally signed stake delegate', () => { tx.addSignature(from.publicKey, signature); expect(tx.verifySignatures()).toBe(true); }); + +test('externally signed stake delegate', () => { + const from_keypair = nacl.sign.keyPair.fromSeed( + Uint8Array.from(Array(32).fill(1)), + ); + const authority = new Account(Buffer.from(from_keypair.secretKey)); + const stake = new PublicKey(2); + const recentBlockhash = new PublicKey(3).toBuffer(); + const vote = new PublicKey(4); + var tx = StakeProgram.delegate({ + stakePubkey: stake, + authorizedPubkey: authority.publicKey, + votePubkey: vote, + }); + const from = authority; + tx.recentBlockhash = bs58.encode(recentBlockhash); + tx.feePayer = from.publicKey; + const tx_bytes = tx.serializeMessage(); + const signature = nacl.sign.detached(tx_bytes, from.secretKey); + tx.addSignature(from.publicKey, signature); + expect(tx.verifySignatures()).toBe(true); +});