fix: always use the nonce as the recent blockhash; never overwrite it (#25829)

Author: steveluscher

src/transaction.ts

@@ -327,17 +327,24 @@ export class Transaction {
       return this._message;
     }
 
-    const {nonceInfo} = this;
-    if (nonceInfo && this.instructions[0] != nonceInfo.nonceInstruction) {
-      this.recentBlockhash = nonceInfo.nonce;
-      this.instructions.unshift(nonceInfo.nonceInstruction);
+    let recentBlockhash;
+    let instructions: TransactionInstruction[];
+    if (this.nonceInfo) {
+      recentBlockhash = this.nonceInfo.nonce;
+      if (this.instructions[0] != this.nonceInfo.nonceInstruction) {
+        instructions = [this.nonceInfo.nonceInstruction, ...this.instructions];
+      } else {
+        instructions = this.instructions;
+      }
+    } else {
+      recentBlockhash = this.recentBlockhash;
+      instructions = this.instructions;
     }
-    const {recentBlockhash} = this;
     if (!recentBlockhash) {
       throw new Error('Transaction recentBlockhash required');
     }
 
-    if (this.instructions.length < 1) {
+    if (instructions.length < 1) {
       console.warn('No instructions provided');
     }
 
@@ -351,8 +358,8 @@ export class Transaction {
       throw new Error('Transaction fee payer required');
     }
 
-    for (let i = 0; i < this.instructions.length; i++) {
-      if (this.instructions[i].programId === undefined) {
+    for (let i = 0; i < instructions.length; i++) {
+      if (instructions[i].programId === undefined) {
         throw new Error(
           `Transaction instruction index ${i} has undefined program id`,
         );
@@ -361,7 +368,7 @@ export class Transaction {
 
     const programIds: string[] = [];
     const accountMetas: AccountMeta[] = [];
-    this.instructions.forEach(instruction => {
+    instructions.forEach(instruction => {
       instruction.keys.forEach(accountMeta => {
         accountMetas.push({...accountMeta});
       });
@@ -471,7 +478,7 @@ export class Transaction {
     });
 
     const accountKeys = signedKeys.concat(unsignedKeys);
-    const instructions: CompiledInstruction[] = this.instructions.map(
+    const compiledInstructions: CompiledInstruction[] = instructions.map(
       instruction => {
         const {data, programId} = instruction;
         return {
@@ -484,7 +491,7 @@ export class Transaction {
       },
     );
 
-    instructions.forEach(instruction => {
+    compiledInstructions.forEach(instruction => {
       invariant(instruction.programIdIndex >= 0);
       instruction.accounts.forEach(keyIndex => invariant(keyIndex >= 0));
     });
@@ -497,7 +504,7 @@ export class Transaction {
       },
       accountKeys,
       recentBlockhash,
-      instructions,
+      instructions: compiledInstructions,
     });
   }
 

test/transaction.test.ts

@@ -230,6 +230,122 @@ describe('Transaction', () => {
       expect(message.header.numReadonlySignedAccounts).to.eq(0);
       expect(message.header.numReadonlyUnsignedAccounts).to.eq(1);
     });
+
+    it('uses the nonce as the recent blockhash when compiling nonce-based transactions', () => {
+      const nonce = new PublicKey(1);
+      const nonceAuthority = new PublicKey(2);
+      const nonceInfo = {
+        nonce: nonce.toBase58(),
+        nonceInstruction: SystemProgram.nonceAdvance({
+          noncePubkey: nonce,
+          authorizedPubkey: nonceAuthority,
+        }),
+      };
+      const transaction = new Transaction({
+        feePayer: nonceAuthority,
+        nonceInfo,
+      });
+      const message = transaction.compileMessage();
+      expect(message.recentBlockhash).to.equal(nonce.toBase58());
+    });
+
+    it('prepends the nonce advance instruction when compiling nonce-based transactions', () => {
+      const nonce = new PublicKey(1);
+      const nonceAuthority = new PublicKey(2);
+      const nonceInfo = {
+        nonce: nonce.toBase58(),
+        nonceInstruction: SystemProgram.nonceAdvance({
+          noncePubkey: nonce,
+          authorizedPubkey: nonceAuthority,
+        }),
+      };
+      const transaction = new Transaction({
+        feePayer: nonceAuthority,
+        nonceInfo,
+      }).add(
+        SystemProgram.transfer({
+          fromPubkey: nonceAuthority,
+          lamports: 1,
+          toPubkey: new PublicKey(3),
+        }),
+      );
+      const message = transaction.compileMessage();
+      expect(message.instructions).to.have.length(2);
+      const expectedNonceAdvanceCompiledInstruction = {
+        accounts: [1, 4, 0],
+        data: (() => {
+          const expectedData = Buffer.alloc(4);
+          expectedData.writeInt32LE(
+            4 /* SystemInstruction::AdvanceNonceAccount */,
+            0,
+          );
+          return bs58.encode(expectedData);
+        })(),
+        programIdIndex: (() => {
+          let foundIndex = -1;
+          message.accountKeys.find((publicKey, ii) => {
+            if (publicKey.equals(SystemProgram.programId)) {
+              foundIndex = ii;
+              return true;
+            }
+          });
+          return foundIndex;
+        })(),
+      };
+      expect(message.instructions[0]).to.deep.equal(
+        expectedNonceAdvanceCompiledInstruction,
+      );
+    });
+
+    it('does not prepend the nonce advance instruction when compiling nonce-based transactions if it is already there', () => {
+      const nonce = new PublicKey(1);
+      const nonceAuthority = new PublicKey(2);
+      const nonceInfo = {
+        nonce: nonce.toBase58(),
+        nonceInstruction: SystemProgram.nonceAdvance({
+          noncePubkey: nonce,
+          authorizedPubkey: nonceAuthority,
+        }),
+      };
+      const transaction = new Transaction({
+        feePayer: nonceAuthority,
+        nonceInfo,
+      })
+        .add(nonceInfo.nonceInstruction)
+        .add(
+          SystemProgram.transfer({
+            fromPubkey: nonceAuthority,
+            lamports: 1,
+            toPubkey: new PublicKey(3),
+          }),
+        );
+      const message = transaction.compileMessage();
+      expect(message.instructions).to.have.length(2);
+      const expectedNonceAdvanceCompiledInstruction = {
+        accounts: [1, 4, 0],
+        data: (() => {
+          const expectedData = Buffer.alloc(4);
+          expectedData.writeInt32LE(
+            4 /* SystemInstruction::AdvanceNonceAccount */,
+            0,
+          );
+          return bs58.encode(expectedData);
+        })(),
+        programIdIndex: (() => {
+          let foundIndex = -1;
+          message.accountKeys.find((publicKey, ii) => {
+            if (publicKey.equals(SystemProgram.programId)) {
+              foundIndex = ii;
+              return true;
+            }
+          });
+          return foundIndex;
+        })(),
+      };
+      expect(message.instructions[0]).to.deep.equal(
+        expectedNonceAdvanceCompiledInstruction,
+      );
+    });
   });
 
   if (process.env.TEST_LIVE) {
@@ -455,15 +571,8 @@ describe('Transaction', () => {
     );
     transferTransaction.sign(account1);
 
-    let expectedData = Buffer.alloc(4);
-    expectedData.writeInt32LE(4, 0);
-
-    expect(transferTransaction.instructions).to.have.length(2);
-    expect(transferTransaction.instructions[0].programId).to.eql(
-      SystemProgram.programId,
-    );
-    expect(transferTransaction.instructions[0].data).to.eql(expectedData);
-    expect(transferTransaction.recentBlockhash).to.eq(nonce);
+    expect(transferTransaction.instructions).to.have.length(1);
+    expect(transferTransaction.recentBlockhash).to.be.undefined;
 
     const stakeAccount = Keypair.generate();
     const voteAccount = Keypair.generate();
@@ -476,12 +585,8 @@ describe('Transaction', () => {
     );
     stakeTransaction.sign(account1);
 
-    expect(stakeTransaction.instructions).to.have.length(2);
-    expect(stakeTransaction.instructions[0].programId).to.eql(
-      SystemProgram.programId,
-    );
-    expect(stakeTransaction.instructions[0].data).to.eql(expectedData);
-    expect(stakeTransaction.recentBlockhash).to.eq(nonce);
+    expect(stakeTransaction.instructions).to.have.length(1);
+    expect(stakeTransaction.recentBlockhash).to.be.undefined;
   });
 
   it('parse wire format and serialize', () => {
@@ -596,6 +701,22 @@ describe('Transaction', () => {
     expect(compiledMessage3).not.to.eql(message);
   });
 
+  it('constructs a transaction with nonce info', () => {
+    const nonce = new PublicKey(1);
+    const nonceAuthority = new PublicKey(2);
+    const nonceInfo = {
+      nonce: nonce.toBase58(),
+      nonceInstruction: SystemProgram.nonceAdvance({
+        noncePubkey: nonce,
+        authorizedPubkey: nonceAuthority,
+      }),
+    };
+    const transaction = new Transaction({nonceInfo});
+    expect(transaction.recentBlockhash).to.be.undefined;
+    expect(transaction.lastValidBlockHeight).to.be.undefined;
+    expect(transaction.nonceInfo).to.equal(nonceInfo);
+  });
+
   it('constructs a transaction with last valid block height', () => {
     const blockhash = 'EETubP5AKHgjPAhzPAFcb8BAY1hMH639CWCFTqi3hq1k';
     const lastValidBlockHeight = 1234;