From 4ea39c1cd3481b76d0aab693ce447635953378f8 Mon Sep 17 00:00:00 2001 From: Steven Luscher Date: Sat, 14 May 2022 21:54:12 -0700 Subject: [PATCH] feat: thread new blockheight expiry strategy through `sendAndConfirmTransaction` (#25227) * chore: extract expirable blockhash record into its own type * fix: the local latest blockhash cache now fetches and stores lastValidBlockHeight * fix: allow people to supply a confirmation strategy to sendAndConfirmRawTransaction * test: upgrade RPC helpers to use blockheight confirmation method * test: patch up tests to use blockheight based confirmation strategy * fix: eliminate deprecated construction of Transaction inside simulateTransaction * test: eliminate deprecated constructions of Transaction in tests --- web3.js/src/connection.ts | 67 ++++++++++------- .../util/send-and-confirm-raw-transaction.ts | 59 +++++++++++++-- web3.js/test/bpf-loader.test.ts | 6 +- web3.js/test/connection.test.ts | 16 +++- web3.js/test/mocks/rpc-http.ts | 8 +- web3.js/test/stake-program.test.ts | 14 ++-- web3.js/test/transaction.test.ts | 74 +++++++++++++------ 7 files changed, 172 insertions(+), 72 deletions(-) diff --git a/web3.js/src/connection.ts b/web3.js/src/connection.ts index 44b7a2a9ef..ed186261ef 100644 --- a/web3.js/src/connection.ts +++ b/web3.js/src/connection.ts @@ -284,15 +284,18 @@ export type RpcResponseAndContext = { value: T; }; +export type BlockhashWithExpiryBlockHeight = Readonly<{ + blockhash: Blockhash; + lastValidBlockHeight: number; +}>; + /** * A strategy for confirming transactions that uses the last valid * block height for a given blockhash to check for transaction expiration. */ export type BlockheightBasedTransactionConfimationStrategy = { signature: TransactionSignature; - blockhash: Blockhash; - lastValidBlockHeight: number; -}; +} & BlockhashWithExpiryBlockHeight; /** * @internal @@ -2218,12 +2221,12 @@ export class Connection { /** @internal */ _disableBlockhashCaching: boolean = false; /** @internal */ _pollingBlockhash: boolean = false; /** @internal */ _blockhashInfo: { - recentBlockhash: Blockhash | null; + latestBlockhash: BlockhashWithExpiryBlockHeight | null; lastFetch: number; simulatedSignatures: Array; transactionSignatures: Array; } = { - recentBlockhash: null, + latestBlockhash: null, lastFetch: 0, transactionSignatures: [], simulatedSignatures: [], @@ -3322,11 +3325,11 @@ export class Connection { /** * Fetch the latest blockhash from the cluster - * @return {Promise<{blockhash: Blockhash, lastValidBlockHeight: number}>} + * @return {Promise} */ async getLatestBlockhash( commitment?: Commitment, - ): Promise<{blockhash: Blockhash; lastValidBlockHeight: number}> { + ): Promise { try { const res = await this.getLatestBlockhashAndContext(commitment); return res.value; @@ -3337,13 +3340,11 @@ export class Connection { /** * Fetch the latest blockhash from the cluster - * @return {Promise<{blockhash: Blockhash, lastValidBlockHeight: number}>} + * @return {Promise} */ async getLatestBlockhashAndContext( commitment?: Commitment, - ): Promise< - RpcResponseAndContext<{blockhash: Blockhash; lastValidBlockHeight: number}> - > { + ): Promise> { const args = this._buildArgs([], commitment); const unsafeRes = await this._rpcRequest('getLatestBlockhash', args); const res = create(unsafeRes, GetLatestBlockhashRpcResult); @@ -3989,7 +3990,9 @@ export class Connection { /** * @internal */ - async _recentBlockhash(disableCache: boolean): Promise { + async _blockhashWithExpiryBlockHeight( + disableCache: boolean, + ): Promise { if (!disableCache) { // Wait for polling to finish while (this._pollingBlockhash) { @@ -3997,8 +4000,8 @@ export class Connection { } const timeSinceFetch = Date.now() - this._blockhashInfo.lastFetch; const expired = timeSinceFetch >= BLOCKHASH_CACHE_TIMEOUT_MS; - if (this._blockhashInfo.recentBlockhash !== null && !expired) { - return this._blockhashInfo.recentBlockhash; + if (this._blockhashInfo.latestBlockhash !== null && !expired) { + return this._blockhashInfo.latestBlockhash; } } @@ -4008,21 +4011,25 @@ export class Connection { /** * @internal */ - async _pollNewBlockhash(): Promise { + async _pollNewBlockhash(): Promise { this._pollingBlockhash = true; try { const startTime = Date.now(); + const cachedLatestBlockhash = this._blockhashInfo.latestBlockhash; + const cachedBlockhash = cachedLatestBlockhash + ? cachedLatestBlockhash.blockhash + : null; for (let i = 0; i < 50; i++) { - const {blockhash} = await this.getRecentBlockhash('finalized'); + const latestBlockhash = await this.getLatestBlockhash('finalized'); - if (this._blockhashInfo.recentBlockhash != blockhash) { + if (cachedBlockhash !== latestBlockhash.blockhash) { this._blockhashInfo = { - recentBlockhash: blockhash, + latestBlockhash, lastFetch: Date.now(), transactionSignatures: [], simulatedSignatures: [], }; - return blockhash; + return latestBlockhash; } // Sleep for approximately half a slot @@ -4048,13 +4055,11 @@ export class Connection { let transaction; if (transactionOrMessage instanceof Transaction) { let originalTx: Transaction = transactionOrMessage; - transaction = new Transaction({ - recentBlockhash: originalTx.recentBlockhash, - nonceInfo: originalTx.nonceInfo, - feePayer: originalTx.feePayer, - signatures: [...originalTx.signatures], - }); + transaction = new Transaction(); + transaction.feePayer = originalTx.feePayer; transaction.instructions = transactionOrMessage.instructions; + transaction.nonceInfo = originalTx.nonceInfo; + transaction.signatures = originalTx.signatures; } else { transaction = Transaction.populate(transactionOrMessage); // HACK: this function relies on mutating the populated transaction @@ -4066,7 +4071,11 @@ export class Connection { } else { let disableCache = this._disableBlockhashCaching; for (;;) { - transaction.recentBlockhash = await this._recentBlockhash(disableCache); + const latestBlockhash = await this._blockhashWithExpiryBlockHeight( + disableCache, + ); + transaction.lastValidBlockHeight = latestBlockhash.lastValidBlockHeight; + transaction.recentBlockhash = latestBlockhash.blockhash; if (!signers) break; @@ -4154,7 +4163,11 @@ export class Connection { } else { let disableCache = this._disableBlockhashCaching; for (;;) { - transaction.recentBlockhash = await this._recentBlockhash(disableCache); + const latestBlockhash = await this._blockhashWithExpiryBlockHeight( + disableCache, + ); + transaction.lastValidBlockHeight = latestBlockhash.lastValidBlockHeight; + transaction.recentBlockhash = latestBlockhash.blockhash; transaction.sign(...signers); if (!transaction.signature) { throw new Error('!signature'); // should never happen diff --git a/web3.js/src/util/send-and-confirm-raw-transaction.ts b/web3.js/src/util/send-and-confirm-raw-transaction.ts index 0f97f577d5..f57021437c 100644 --- a/web3.js/src/util/send-and-confirm-raw-transaction.ts +++ b/web3.js/src/util/send-and-confirm-raw-transaction.ts @@ -1,6 +1,9 @@ import type {Buffer} from 'buffer'; -import {Connection} from '../connection'; +import { + BlockheightBasedTransactionConfimationStrategy, + Connection, +} from '../connection'; import type {TransactionSignature} from '../transaction'; import type {ConfirmOptions} from '../connection'; @@ -11,14 +14,57 @@ import type {ConfirmOptions} from '../connection'; * * @param {Connection} connection * @param {Buffer} rawTransaction + * @param {BlockheightBasedTransactionConfimationStrategy} confirmationStrategy * @param {ConfirmOptions} [options] * @returns {Promise} */ export async function sendAndConfirmRawTransaction( connection: Connection, rawTransaction: Buffer, + confirmationStrategy: BlockheightBasedTransactionConfimationStrategy, options?: ConfirmOptions, +): Promise; + +/** + * @deprecated Calling `sendAndConfirmRawTransaction()` without a `confirmationStrategy` + * is no longer supported and will be removed in a future version. + */ +// eslint-disable-next-line no-redeclare +export async function sendAndConfirmRawTransaction( + connection: Connection, + rawTransaction: Buffer, + options?: ConfirmOptions, +): Promise; + +// eslint-disable-next-line no-redeclare +export async function sendAndConfirmRawTransaction( + connection: Connection, + rawTransaction: Buffer, + confirmationStrategyOrConfirmOptions: + | BlockheightBasedTransactionConfimationStrategy + | ConfirmOptions + | undefined, + maybeConfirmOptions?: ConfirmOptions, ): Promise { + let confirmationStrategy: + | BlockheightBasedTransactionConfimationStrategy + | undefined; + let options: ConfirmOptions | undefined; + if ( + confirmationStrategyOrConfirmOptions && + Object.prototype.hasOwnProperty.call( + confirmationStrategyOrConfirmOptions, + 'lastValidBlockHeight', + ) + ) { + confirmationStrategy = + confirmationStrategyOrConfirmOptions as BlockheightBasedTransactionConfimationStrategy; + options = maybeConfirmOptions; + } else { + options = confirmationStrategyOrConfirmOptions as + | ConfirmOptions + | undefined; + } const sendOptions = options && { skipPreflight: options.skipPreflight, preflightCommitment: options.preflightCommitment || options.commitment, @@ -29,12 +75,11 @@ export async function sendAndConfirmRawTransaction( sendOptions, ); - const status = ( - await connection.confirmTransaction( - signature, - options && options.commitment, - ) - ).value; + const commitment = options && options.commitment; + const confirmationPromise = confirmationStrategy + ? connection.confirmTransaction(confirmationStrategy, commitment) + : connection.confirmTransaction(signature, commitment); + const status = (await confirmationPromise).value; if (status.err) { throw new Error( diff --git a/web3.js/test/bpf-loader.test.ts b/web3.js/test/bpf-loader.test.ts index 43c50e1b1c..4d4d5e820f 100644 --- a/web3.js/test/bpf-loader.test.ts +++ b/web3.js/test/bpf-loader.test.ts @@ -176,9 +176,9 @@ if (process.env.TEST_LIVE) { }); it('simulate transaction without signature verification', async () => { - const simulatedTransaction = new Transaction({ - feePayer: payerAccount.publicKey, - }).add({ + const simulatedTransaction = new Transaction(); + simulatedTransaction.feePayer = payerAccount.publicKey; + simulatedTransaction.add({ keys: [ {pubkey: payerAccount.publicKey, isSigner: true, isWritable: true}, ], diff --git a/web3.js/test/connection.test.ts b/web3.js/test/connection.test.ts index 391e69bfe1..2007c7e99e 100644 --- a/web3.js/test/connection.test.ts +++ b/web3.js/test/connection.test.ts @@ -1131,7 +1131,11 @@ describe('Connection', function () { const badTransactionSignature = 'bad transaction signature'; await expect( - connection.confirmTransaction(badTransactionSignature), + connection.confirmTransaction({ + blockhash: 'sampleBlockhash', + lastValidBlockHeight: 9999, + signature: badTransactionSignature, + }), ).to.be.rejectedWith('signature must be base58 encoded'); await mockRpcResponse({ @@ -2899,11 +2903,11 @@ describe('Connection', function () { const accountFrom = Keypair.generate(); const accountTo = Keypair.generate(); - const {blockhash} = await helpers.latestBlockhash({connection}); + const latestBlockhash = await helpers.latestBlockhash({connection}); const transaction = new Transaction({ feePayer: accountFrom.publicKey, - recentBlockhash: blockhash, + ...latestBlockhash, }).add( SystemProgram.transfer({ fromPubkey: accountFrom.publicKey, @@ -3825,7 +3829,11 @@ describe('Connection', function () { {skipPreflight: true}, ); - await connection.confirmTransaction(signature); + await connection.confirmTransaction({ + blockhash: transaction.recentBlockhash, + lastValidBlockHeight: transaction.lastValidBlockHeight, + signature, + }); const response = (await connection.getSignatureStatus(signature)).value; if (response !== null) { diff --git a/web3.js/test/mocks/rpc-http.ts b/web3.js/test/mocks/rpc-http.ts index 149f9b4dab..c0735327b2 100644 --- a/web3.js/test/mocks/rpc-http.ts +++ b/web3.js/test/mocks/rpc-http.ts @@ -185,7 +185,8 @@ const processTransaction = async ({ commitment: Commitment; err?: any; }) => { - const blockhash = (await latestBlockhash({connection})).blockhash; + const {blockhash, lastValidBlockHeight} = await latestBlockhash({connection}); + transaction.lastValidBlockHeight = lastValidBlockHeight; transaction.recentBlockhash = blockhash; transaction.sign(...signers); @@ -222,7 +223,10 @@ const processTransaction = async ({ result: true, }); - return await connection.confirmTransaction(signature, commitment); + return await connection.confirmTransaction( + {blockhash, lastValidBlockHeight, signature}, + commitment, + ); }; const airdrop = async ({ diff --git a/web3.js/test/stake-program.test.ts b/web3.js/test/stake-program.test.ts index cd4ebfad3b..f7bfee3271 100644 --- a/web3.js/test/stake-program.test.ts +++ b/web3.js/test/stake-program.test.ts @@ -339,9 +339,10 @@ describe('StakeProgram', () => { lockup: new Lockup(0, 0, from.publicKey), lamports: amount, }); - const createWithSeedTransaction = new Transaction({recentBlockhash}).add( - createWithSeed, - ); + const createWithSeedTransaction = new Transaction({ + blockhash: recentBlockhash, + lastValidBlockHeight: 9999, + }).add(createWithSeed); expect(createWithSeedTransaction.instructions).to.have.length(2); const systemInstructionType = SystemInstruction.decodeInstructionType( @@ -368,9 +369,10 @@ describe('StakeProgram', () => { votePubkey: vote.publicKey, }); - const delegateTransaction = new Transaction({recentBlockhash}).add( - delegate, - ); + const delegateTransaction = new Transaction({ + blockhash: recentBlockhash, + lastValidBlockHeight: 9999, + }).add(delegate); const anotherStakeInstructionType = StakeInstruction.decodeInstructionType( delegateTransaction.instructions[0], ); diff --git a/web3.js/test/transaction.test.ts b/web3.js/test/transaction.test.ts index 73e3eab6c1..37dd0d7df9 100644 --- a/web3.js/test/transaction.test.ts +++ b/web3.js/test/transaction.test.ts @@ -23,7 +23,10 @@ describe('Transaction', () => { const account3 = Keypair.generate(); const recentBlockhash = Keypair.generate().publicKey.toBase58(); const programId = Keypair.generate().publicKey; - const transaction = new Transaction({recentBlockhash}).add({ + const transaction = new Transaction({ + blockhash: recentBlockhash, + lastValidBlockHeight: 9999, + }).add({ keys: [ {pubkey: account3.publicKey, isSigner: true, isWritable: false}, {pubkey: payer.publicKey, isSigner: true, isWritable: true}, @@ -49,7 +52,10 @@ describe('Transaction', () => { const other = Keypair.generate(); const recentBlockhash = Keypair.generate().publicKey.toBase58(); const programId = Keypair.generate().publicKey; - const transaction = new Transaction({recentBlockhash}).add({ + const transaction = new Transaction({ + blockhash: recentBlockhash, + lastValidBlockHeight: 9999, + }).add({ keys: [ {pubkey: other.publicKey, isSigner: true, isWritable: true}, {pubkey: payer.publicKey, isSigner: true, isWritable: true}, @@ -101,7 +107,10 @@ describe('Transaction', () => { const payer = Keypair.generate(); const recentBlockhash = Keypair.generate().publicKey.toBase58(); const programId = Keypair.generate().publicKey; - const transaction = new Transaction({recentBlockhash}).add({ + const transaction = new Transaction({ + blockhash: recentBlockhash, + lastValidBlockHeight: 9999, + }).add({ keys: [{pubkey: payer.publicKey, isSigner: true, isWritable: false}], programId, }); @@ -121,11 +130,11 @@ describe('Transaction', () => { const accountFrom = Keypair.generate(); const accountTo = Keypair.generate(); - const {blockhash} = await helpers.latestBlockhash({connection}); + const latestBlockhash = await helpers.latestBlockhash({connection}); const transaction = new Transaction({ feePayer: accountFrom.publicKey, - recentBlockhash: blockhash, + ...latestBlockhash, }).add( SystemProgram.transfer({ fromPubkey: accountFrom.publicKey, @@ -149,10 +158,16 @@ describe('Transaction', () => { lamports: 123, }); - const transaction = new Transaction({recentBlockhash}).add(transfer); + const transaction = new Transaction({ + blockhash: recentBlockhash, + lastValidBlockHeight: 9999, + }).add(transfer); transaction.sign(account1, account2); - const partialTransaction = new Transaction({recentBlockhash}).add(transfer); + const partialTransaction = new Transaction({ + blockhash: recentBlockhash, + lastValidBlockHeight: 9999, + }).add(transfer); partialTransaction.setSigners(account1.publicKey, account2.publicKey); expect(partialTransaction.signatures[0].signature).to.be.null; expect(partialTransaction.signatures[1].signature).to.be.null; @@ -196,7 +211,10 @@ describe('Transaction', () => { const programId = Keypair.generate().publicKey; it('setSigners', () => { - const transaction = new Transaction({recentBlockhash}).add({ + const transaction = new Transaction({ + blockhash: recentBlockhash, + lastValidBlockHeight: 9999, + }).add({ keys: [ {pubkey: duplicate1.publicKey, isSigner: true, isWritable: true}, {pubkey: payer.publicKey, isSigner: false, isWritable: true}, @@ -224,7 +242,10 @@ describe('Transaction', () => { }); it('sign', () => { - const transaction = new Transaction({recentBlockhash}).add({ + const transaction = new Transaction({ + blockhash: recentBlockhash, + lastValidBlockHeight: 9999, + }).add({ keys: [ {pubkey: duplicate1.publicKey, isSigner: true, isWritable: true}, {pubkey: payer.publicKey, isSigner: false, isWritable: true}, @@ -263,14 +284,18 @@ describe('Transaction', () => { lamports: 123, }); - const orgTransaction = new Transaction({recentBlockhash}).add( - transfer1, - transfer2, - ); + const latestBlockhash = { + blockhash: recentBlockhash, + lastValidBlockHeight: 9999, + }; + + const orgTransaction = new Transaction({ + ...latestBlockhash, + }).add(transfer1, transfer2); orgTransaction.sign(account1, account2); const newTransaction = new Transaction({ - recentBlockhash: orgTransaction.recentBlockhash, + ...latestBlockhash, signatures: orgTransaction.signatures, }).add(transfer1, transfer2); @@ -292,10 +317,10 @@ describe('Transaction', () => { lamports: 123, }); - const orgTransaction = new Transaction({recentBlockhash}).add( - transfer1, - transfer2, - ); + const orgTransaction = new Transaction({ + blockhash: recentBlockhash, + lastValidBlockHeight: 9999, + }).add(transfer1, transfer2); orgTransaction.sign(account1); }); @@ -363,8 +388,9 @@ describe('Transaction', () => { lamports: 49, }); const expectedTransaction = new Transaction({ - recentBlockhash, + blockhash: recentBlockhash, feePayer: sender.publicKey, + lastValidBlockHeight: 9999, }).add(transfer); expectedTransaction.sign(sender); @@ -493,9 +519,10 @@ describe('Transaction', () => { toPubkey: recipient, lamports: 49, }); - const expectedTransaction = new Transaction({recentBlockhash}).add( - transfer, - ); + const expectedTransaction = new Transaction({ + blockhash: recentBlockhash, + lastValidBlockHeight: 9999, + }).add(transfer); // Empty signature array fails. expect(expectedTransaction.signatures).to.have.length(0); @@ -607,8 +634,9 @@ describe('Transaction', () => { const acc1Writable = Keypair.generate(); const acc2Writable = Keypair.generate(); const t0 = new Transaction({ - recentBlockhash: 'HZaTsZuhN1aaz9WuuimCFMyH7wJ5xiyMUHFCnZSMyguH', + blockhash: 'HZaTsZuhN1aaz9WuuimCFMyH7wJ5xiyMUHFCnZSMyguH', feePayer: signer.publicKey, + lastValidBlockHeight: 9999, }); t0.add( new TransactionInstruction({