Skip to content

Commit

Permalink
fix: always use the nonce as the recent blockhash; never overwrite it (…
Browse files Browse the repository at this point in the history 
…#25829)
  • Loading branch information
@steveluscher
steveluscher committed Jun 27, 2022
1 parent cd2878a commit a741edd
Show file tree
Hide file tree
Showing 2 changed files with 155 additions and 27 deletions.
31 changes: 19 additions & 12 deletions web3.js/src/transaction.ts
View file
Expand Up @@ -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');
}

Expand All @@ -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`,
);
Expand All @@ -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});
});
Expand Down Expand Up @@ -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 {
Expand All @@ -484,7 +491,7 @@ export class Transaction {
},
);

instructions.forEach(instruction => {
compiledInstructions.forEach(instruction => {
invariant(instruction.programIdIndex >= 0);
instruction.accounts.forEach(keyIndex => invariant(keyIndex >= 0));
});
Expand All @@ -497,7 +504,7 @@ export class Transaction {
},
accountKeys,
recentBlockhash,
instructions,
instructions: compiledInstructions,
});
}

Expand Down
151 changes: 136 additions & 15 deletions web3.js/test/transaction.test.ts
View file
Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit a741edd

Please sign in to comment.