Use latest stable validator version in CI#1544
Conversation
🦋 Changeset detectedLatest commit: 8501822 The changes in this PR will be included in the next version bump. This PR includes changesets to release 46 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
BundleMonUnchanged files (144)
No change in files bundle size Final result: ✅ View report in BundleMon website ➡️ |
trevor-cortex
left a comment
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
CI was broken because the GitHub releases API pagination no longer returns any 2.3.x versions on the first page. This PR fixes it by switching from filtering for v2.3.* to finding the latest stable (non-prerelease, no alpha/beta/rc in tag name) release instead.
Notes
This looks good to me! A couple of small things to be aware of:
- The regex
/alpha|beta|rc/doesn't filter outtestversions, which was mentioned in the PR description. Not sure if that's intentional or if it just doesn't exist in practice. Might be worth adding it for safety. - This still only looks at the first page of results (30 items by default from the GitHub API). If all 30 happen to be prereleases at some point, the script will fall through to the cache-busting path. Unlikely in practice, but worth noting.
- The
findapproach (first match) works well here since GitHub returns releases in reverse chronological order.
| set -e | ||
| version=$(node -e \ | ||
| 'fetch("https://api.github.com/repos/anza-xyz/agave/releases").then(res => res.json().then(rs => rs.filter(r => !r.prerelease && r.tag_name.startsWith("v2.3."))).then(x => console.log(x[0].tag_name)));' | ||
| 'fetch("https://api.github.com/repos/anza-xyz/agave/releases").then(res => res.json()).then(rs => { const r = rs.find(r => !r.prerelease && !/alpha|beta|rc/.test(r.tag_name)); if (r) console.log(r.tag_name); });' |
There was a problem hiding this comment.
Nit: the PR description mentions filtering out test versions too, but the regex only covers alpha|beta|rc. Worth adding test to the pattern?
| 'fetch("https://api.github.com/repos/anza-xyz/agave/releases").then(res => res.json()).then(rs => { const r = rs.find(r => !r.prerelease && !/alpha|beta|rc/.test(r.tag_name)); if (r) console.log(r.tag_name); });' | |
| 'fetch("https://api.github.com/repos/anza-xyz/agave/releases").then(res => res.json()).then(rs => { const r = rs.find(r => !r.prerelease && !/alpha|beta|rc|test/.test(r.tag_name)); if (r) console.log(r.tag_name); });' |
There was a problem hiding this comment.
oops misread the regex while writing the PR description, but we don't use -test in tag names so not needed. Last 100 releases have only used alpha/beta.
|
Documentation Preview: https://kit-docs-cp9mjq6ef-anza-tech.vercel.app |
mcintyre94
force-pushed
the
validator-version
branch
from
6f2d9a1 to
da9172b
Compare
| ( | ||
| if flock -nx 200; then | ||
| $TEST_VALIDATOR --ledger $TEST_VALIDATOR_LEDGER --reset --quiet --account-dir $FIXTURE_ACCOUNTS_DIR --rpc-pubsub-enable-vote-subscription --rpc-pubsub-enable-block-subscription >/dev/null & | ||
| $TEST_VALIDATOR --ledger $TEST_VALIDATOR_LEDGER --reset --quiet --account-dir $FIXTURE_ACCOUNTS_DIR --rpc-pubsub-enable-vote-subscription >/dev/null & |
There was a problem hiding this comment.
Seems like this flag has been removed, caused an error on startup
There was a problem hiding this comment.
I ran into the exact same issue yesterday (wallet-ui repo is a fork of this base repo)
There was a problem hiding this comment.
Thanks @beeman! I'm hoping this PR pinning to the latest stable release is going to be the right longer term fix here
mcintyre94
force-pushed
the
validator-version
branch
from
bf3cb40 to
76b85b0
Compare
mcintyre94
force-pushed
the
validator-version
branch
from
9c69129 to
3d76134
Compare
trevor-cortex
left a comment
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
This PR fixes broken CI by switching from the pinned v2.3.* validator version to the latest stable release (currently 3.1.13). It also updates all affected types and tests to match the 3.1.13 validator's responses, including:
- New fields on
SimulateTransactionApiResponseBase:fee,loadedAddresses,preBalances,postBalances,preTokenBalances,postTokenBalances, andreplacementBlockhash(now always present, nullable). - New fields on
JsonParsedVoteAccount:blockRevenueCollector,blockRevenueCommissionBps,blsPubkeyCompressed,inflationRewardsCollector,inflationRewardsCommissionBps,pendingDelegatorRewards, andlatencyon votes. - New fields on
RpcSimulateTransactionResult(errors package) to stay in sync with the RPC API types. - Test assertions relaxed to use
expect.objectContaining()/expect.arrayContaining()to accommodate new fields from the validator without brittle exact matching. - Behavioral change: invalid signature with
sigVerify: truenow returns a result witherr: 'SignatureFailure'instead of throwingSERVER_ERROR_TRANSACTION_SIGNATURE_VERIFICATION_FAILURE. - Removed
--rpc-pubsub-enable-block-subscriptionflag from the test validator start script (presumably dropped in 3.x).
Key things to watch
- The
replacementBlockhashtype change is a breaking change for consumers: it was previously conditional onreplaceRecentBlockhashconfig, but is now always present asT | nullin the base response. The typetest update reflects this. Downstream users who relied onreplacementBlockhashnot existing in the response type whenreplaceRecentBlockhashis false will need to handle thenullcase. - The
SimulateTransactionApiResponseWithReplacementBlockhashtype still narrowsreplacementBlockhashto non-nullTransactionBlockhashLifetimefor thereplaceRecentBlockhash: trueoverloads — this is correct since&will intersect to the non-null version. - The signature verification behavior change (returning
SignatureFailurein the result rather than throwing) seems like a validator-level change. Worth documenting if this affects any public error handling guidance.
Notes for subsequent reviewers
- Verify that all new
JsonParsedVoteAccountfields match the actual Agave 3.1.13 vote account schema. The fields look reasonable (BLS key support, revenue collectors, delegator rewards) but worth cross-referencing with Agave source. - The
postTokenBalancesandpreTokenBalancesare typed asunknown[]inRpcSimulateTransactionResult(errors package) but asreadonly TokenBalance[]insimulateTransaction.ts— this is fine since the errors package is looser by design, but worth noting the asymmetry. - The
loadedAccountsDataSizefield is now optional (?:) inSimulateTransactionApiResponseBase— check if 3.1.13 still returns it or if it's been removed.
LGTM 👍
| set -e | ||
| version=$(node -e \ | ||
| 'fetch("https://api.github.com/repos/anza-xyz/agave/releases").then(res => res.json().then(rs => rs.filter(r => !r.prerelease && r.tag_name.startsWith("v2.3."))).then(x => console.log(x[0].tag_name)));' | ||
| 'fetch("https://api.github.com/repos/anza-xyz/agave/releases").then(res => res.json()).then(rs => { const r = rs.find(r => !r.prerelease && !/alpha|beta|rc/.test(r.tag_name)); if (r) console.log(r.tag_name); });' |
There was a problem hiding this comment.
Previous review noted this: the regex filters alpha|beta|rc but not test (which was mentioned in the original PR description). Looks like the description was updated to drop test — just confirming that's intentional and not an oversight.
| replacementBlockhash: TransactionBlockhashLifetime | null; | ||
| /** The most-recent return data generated by an instruction in the transaction */ | ||
| returnData: Readonly<{ | ||
| /** The return data itself, as base-64 encoded binary data */ |
There was a problem hiding this comment.
Nit: replacementBlockhash is now in the base response type as T | null, but SimulateTransactionApiResponseWithReplacementBlockhash still exists and narrows it to non-null. This works correctly via intersection (&), but it does mean replacementBlockhash appears in two places. Might be worth a comment on the base field noting that it gets narrowed by the intersection type for replaceRecentBlockhash: true overloads.
| await expect(resultPromise).rejects.toThrow( | ||
| new SolanaError(SOLANA_ERROR__JSON_RPC__SERVER_ERROR_TRANSACTION_SIGNATURE_VERIFICATION_FAILURE), | ||
| ); | ||
| await expect(resultPromise).resolves.toStrictEqual({ |
There was a problem hiding this comment.
Good catch on this behavioral change. The validator now returns SignatureFailure as a result error rather than throwing a JSON-RPC error. Worth noting in a changelog or migration guide if this repo publishes one — consumers catching SERVER_ERROR_TRANSACTION_SIGNATURE_VERIFICATION_FAILURE will need to update their error handling.
… a breaking change
|
@trevor-cortex Is "The replacementBlockhash type change" the only breaking change? |
|
No, there are several breaking changes in this PR beyond 1. New fields added to
These are added to the base type, so any code doing exhaustive checks on the response shape will see new properties. 2. 3. Signature verification behavior change 4. New fields on
5. Rent sysvar values changed
These are runtime data changes rather than type changes, but they'll break any snapshot/exact-match tests. 6. So TL;DR: the |
mcintyre94
force-pushed
the
validator-version
branch
from
24cda96 to
8501822
Compare
|
I disagree that these are major breaking changes. Importantly, this PR is not upgrading the validator version for consumers, that's already happened and is independent of Kit. We pick up the breaking changes from the validator intentionally in our tests to inform our types.
|
lorisleiva
left a comment
lorisleiva
left a comment
There was a problem hiding this comment.
Amazing! Tysm for keeping up with these changes. 🫶
|
🔎💬 Inkeep AI search and chat service is syncing content for source 'Solana Kit Docs' |
|
Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up. |
4 participants
Problem
CI is broken: https://github.com/anza-xyz/kit/actions/runs/24452992515/job/71487609515?pr=1542
This is because the anza releases page that we use to find the validator version no longer has any 2.3.x versions on the first page of results
Summary of Changes
This PR changes the approach so that we use the latest stable (not prerelease, no alpha/beta/rc in tag name) version. Currently this is 3.1.13, as all 4.x releases are betas.
This might turn out to be a bad approach, but I think it's probably better for our CI to be running against the validator the network is currently using. See https://www.validators.app/ - currently around 73% of the network are on 3.1.13
We also update the types (at least those covered by tests), to match what the 3.1.13 validator is returning. This is purely additive, which justifies a minor changeset. Our tests do validate precise values (to catch fields missing from our types) and so needed to be updated. Adding new fields to types is not a breaking change in Kit though.
There is one exception: The RPC now returns
replacementBlockhash: nullwhenreplaceRecentBlockhashis false in simulateTransaction. Previously it excluded the field. This would be a breaking change to Kit types, so we don't include it in this PR to unblock CI. #1546Going forward we can decide at each breaking version whether to pin to eg 3.1.x, or immediately update to keep up with the current stable version. But I think staying permanently pinned to an old version (2.3.x) is the worst of all options. Consumers are going to be receiving responses from a network on 3.1.x currently.
CI run with
--forceto ensure all tests pass: https://github.com/anza-xyz/kit/actions/runs/24504824860/job/71620554038