Skip to content

Commit c801637

Browse files
authored
fix: downshift preflightCommitment to processed when bypassing preflight checks (#2415)
# Summary There's a bug in the `sendTransaction` RPC method where, when bypassing preflight, we nevertheless materially use the value of `preflightCommitment` to determine how to behave when sending the transaction. If you supply _nothing_ – as you might think appropriate when _skipping_ preflight – then the default of `finalized` will be used. Far from irrelevant, such a value can actually affect the retry behaviour of the send-transaction-service (STS). Read anza-xyz/agave#483 for more detail. In this PR, we try to get ahead of anza-xyz/agave#483 by setting this value to `processed` in the client. Until the server PR is deployed widely, this should cover those who choose to upgrade Addresses anza-xyz/agave#479 # Test plan ``` pnpm turbo test:unit:node test:unit:browser ```
1 parent 0be6bed commit c801637

File tree

7 files changed

+81
-7
lines changed

7 files changed

+81
-7
lines changed

.changeset/tricky-fishes-pull.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@solana/rpc-transformers': patch
3+
---
4+
5+
Improve transaction sending reliability for those who skip preflight (simulation) when calling `sendTransaction`

packages/library-legacy/src/connection.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5895,7 +5895,9 @@ export class Connection {
58955895
const config: any = {encoding: 'base64'};
58965896
const skipPreflight = options && options.skipPreflight;
58975897
const preflightCommitment =
5898-
(options && options.preflightCommitment) || this.commitment;
5898+
skipPreflight === true
5899+
? 'processed' // FIXME Remove when https://github.com/anza-xyz/agave/pull/483 is deployed.
5900+
: (options && options.preflightCommitment) || this.commitment;
58995901

59005902
if (options && options.maxRetries != null) {
59015903
config.maxRetries = options.maxRetries;

packages/library-legacy/test/connection.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4968,6 +4968,29 @@ describe('Connection', function () {
49684968
});
49694969
}
49704970

4971+
// FIXME Remove when https://github.com/anza-xyz/agave/pull/483 is deployed.
4972+
(
4973+
[undefined, 'processed', 'confirmed', 'finalized'] as (
4974+
| Commitment
4975+
| undefined
4976+
)[]
4977+
).forEach(explicitPreflightCommitment => {
4978+
it(`sets \`preflightCommitment\` to \`processed\` when \`skipPreflight\` is \`true\`, no matter that \`preflightCommitment\` was set to \`${explicitPreflightCommitment}\``, () => {
4979+
const connection = new Connection(url);
4980+
const rpcRequestMethod = spy(connection, '_rpcRequest');
4981+
connection.sendEncodedTransaction('ENCODEDTRANSACTION', {
4982+
...(explicitPreflightCommitment
4983+
? {preflightCommitment: explicitPreflightCommitment}
4984+
: null),
4985+
skipPreflight: true,
4986+
});
4987+
expect(rpcRequestMethod).to.have.been.calledWithExactly(
4988+
'sendTransaction',
4989+
[match.any, match.has('preflightCommitment', 'processed')],
4990+
);
4991+
});
4992+
});
4993+
49714994
it('get largest accounts', async () => {
49724995
await mockRpcResponse({
49734996
method: 'getLargestAccounts',

packages/rpc-transformers/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
"maintained node versions"
6464
],
6565
"dependencies": {
66+
"@solana/functional": "workspace:*",
6667
"@solana/rpc-types": "workspace:*",
6768
"@solana/rpc-spec": "workspace:*",
6869
"@solana/rpc-subscriptions-spec": "workspace:*"

packages/rpc-transformers/src/__tests__/params-transformer-test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,28 @@ describe('getDefaultParamsTransformerForSolanaRpc', () => {
212212
},
213213
);
214214
});
215+
// FIXME Remove when https://github.com/anza-xyz/agave/pull/483 is deployed.
216+
it.each([undefined, 'processed', 'confirmed', 'finalized'])(
217+
'sets the `preflightCommitment` to `processed` when `skipPreflight` is `true` and `preflightCommitment` is `%s` on calls to `sendTransaction`',
218+
explicitPreflightCommitment => {
219+
const patcher = getDefaultParamsTransformerForSolanaRpc();
220+
expect(
221+
patcher(
222+
[
223+
null,
224+
{
225+
// eslint-disable-next-line jest/no-conditional-in-test
226+
...(explicitPreflightCommitment
227+
? { preflightCommitment: explicitPreflightCommitment }
228+
: null),
229+
skipPreflight: true,
230+
},
231+
],
232+
'sendTransaction',
233+
),
234+
).toContainEqual(expect.objectContaining({ preflightCommitment: 'processed' }));
235+
},
236+
);
215237
describe('given an integer overflow handler', () => {
216238
let onIntegerOverflow: jest.Mock;
217239
let paramsTransformer: (value: unknown) => unknown;

packages/rpc-transformers/src/params-transformer.ts

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { pipe } from '@solana/functional';
12
import { Commitment } from '@solana/rpc-types';
23

34
import { applyDefaultCommitment } from './default-commitment';
@@ -32,11 +33,28 @@ export function getDefaultParamsTransformerForSolanaRpc(config?: ParamsTransform
3233
if (optionsObjectPositionInParams == null) {
3334
return patchedParams;
3435
}
35-
return applyDefaultCommitment({
36-
commitmentPropertyName: methodName === 'sendTransaction' ? 'preflightCommitment' : 'commitment',
37-
optionsObjectPositionInParams,
38-
overrideCommitment: defaultCommitment,
39-
params: patchedParams,
40-
});
36+
return pipe(
37+
patchedParams,
38+
params =>
39+
applyDefaultCommitment({
40+
commitmentPropertyName: methodName === 'sendTransaction' ? 'preflightCommitment' : 'commitment',
41+
optionsObjectPositionInParams,
42+
overrideCommitment: defaultCommitment,
43+
params,
44+
}),
45+
// FIXME Remove when https://github.com/anza-xyz/agave/pull/483 is deployed.
46+
params =>
47+
methodName === 'sendTransaction'
48+
? applyFixForIssue479(params as [unknown, { skipPreflight?: boolean } | undefined])
49+
: params,
50+
);
4151
};
4252
}
53+
54+
// See https://github.com/anza-xyz/agave/issues/479
55+
function applyFixForIssue479(params: [unknown, { skipPreflight?: boolean } | undefined]) {
56+
if (params[1]?.skipPreflight !== true) {
57+
return params;
58+
}
59+
return [params[0], { ...params[1], preflightCommitment: 'processed' }, ...params.slice(2)];
60+
}

pnpm-lock.yaml

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)