Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add isBlockhashValid method to web3.js #25888

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions web3.js/src/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,16 @@ const BlockProductionResponseStruct = jsonRpcResultAndContext(
}),
);

/**
* Configuration object for changing `isBlockhashValid` query behavior
*/
export type IsBlockhashValidConfig = {
/** The level of commitment desired */
commitment?: Commitment;
/** The minimum slot that the request can be evaluated at */
minContextSlot?: number;
}

/**
* A performance sample
*/
Expand Down Expand Up @@ -3359,6 +3369,35 @@ export class Connection {
return res.result;
}

/**
* Check whether a blockhash is still valid or not
*/
async isBlockhashValid(
blockhash: string,
config?: IsBlockhashValidConfig
): Promise<RpcResponseAndContext<boolean>> {
const extra: Pick<IsBlockhashValidConfig, 'minContextSlot'> = {};
zhe-t marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, you want to take an Omit approach rather than a Pick approach.

const extra = Omit<IsBlockhashValidConfig, 'commitment'> = {};


const commitment = config
? config.commitment
: this._commitment || 'finalized';
const args = this._buildArgs(
[blockhash],
commitment,
'jsonParsed',
extra,
);

const unsafeRes = await this._rpcRequest('isBlockhashValid', args);
const res = create(unsafeRes, jsonRpcResultAndContext(boolean()));
if ('error' in res) {
throw new Error(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have special handling for when the min context slot isn't met. It can be detected from its error code:

pub const JSON_RPC_SERVER_ERROR_MIN_CONTEXT_SLOT_NOT_REACHED: i64 = -32016;

We could retry a few times (configurable in IsBlockhashValidConfig). What do you think? I wouldn't mind this change coming later in a separate PR too. So maybe we just ensure that the thrown error object includes the error code as an integer

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat. Two questions come to mind:

  1. Should we check for this error code in res and retry a configurable amount of times before bailing?
  2. Should we return the error code be propagated in the response object of IsBlockhashValid?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. yup, that's what I had in mind
  2. no, it should be abstracted away

Copy link
Author

@zhe-t zhe-t Jul 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall I include this in the current PR? @jstarry @steveluscher

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you made this PR, we introduced SolanaJSONRPCError.

'Could not determine if blockhash ' + blockhash + ' was valid: ' + res.error.message,
);
}
return res.result;
}

/**
* Fetch the node version
*/
Expand Down
55 changes: 55 additions & 0 deletions web3.js/test/connection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2952,6 +2952,61 @@ describe('Connection', function () {
}
});

describe('is blockhash valid', () => {
it('is blockhash valid - blockhash only (mock)', async () => {
const {blockhash} = await helpers.recentBlockhash({connection});

await mockRpcResponse({
method: 'isBlockhashValid',
params: [blockhash],
value: true,
withContext: true
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mockRpcResponse helper actually skips when you're in live mode, so that you only have to write the tests once.


const isValid = await connection.isBlockhashValid(blockhash);
expect(isValid.value).to.be.true;
});

it('is blockhash valid - blockhash and min slot (mock)', async () => {
const {blockhash} = await helpers.recentBlockhash({connection});

await mockRpcResponse({
method: 'isBlockhashValid',
params: [blockhash, {minContextSlot: 2}],
value: true,
withContext: true
});

const isValid = await connection.isBlockhashValid(blockhash, {minContextSlot: 2});
expect(isValid.value).to.be.true;
});

if (process.env.TEST_LIVE) {
it('is blockhash valid - blockhash only (live)', async () => {
const {blockhash} = await helpers.recentBlockhash({connection});
const isValid = await connection.isBlockhashValid(blockhash);
expect(isValid.value).to.be.true;
});

it('is blockhash valid - blockhash only and mint slot (live)', async () => {
const blockhash = await connection.getLatestBlockhashAndContext('finalized');

const isValid = await connection.isBlockhashValid(
blockhash.value.blockhash,
{
commitment: 'finalized',
minContextSlot: Number.MAX_SAFE_INTEGER
});
expect(isValid.value).to.be.false;
});

it('is blockhash valid - junk blockhash (live)', async () => {
const isValid = await connection.isBlockhashValid('57zQNBZBEiHsCZFqsaY6h176ioXy5MsSLmcvHkEyaLGy');
expect(isValid.value).to.be.false;
});
}
});

it('get minimum ledger slot', async () => {
await mockRpcResponse({
method: 'minimumLedgerSlot',
Expand Down