Bump solana-go to 1.23 and resolve breakings - #1631
Conversation
|
📊 API Diff Results
|
📊 API Diff Results
|
| // newer transaction would be parsed with the v0 layout rather than rejected. Until that gains | ||
| // real support, decoding stays capped at MaxSupportTransactionVersion. | ||
| const MaxRequestedTransactionVersion = uint64(1) | ||
| // Bump this once solana-go supports decoding a newer message version. |
There was a problem hiding this comment.
nit: think this comment is stale now since this PR is adding support
toblich
left a comment
toblich
left a comment
There was a problem hiding this comment.
Looking good, just a couple comments
| // solana-go's MarshalBinary pads any missing signatures with zero bytes rather than | ||
| // erroring, so an under-signed transaction would otherwise wire-encode as if it were | ||
| // fully signed and only fail asynchronously once the cluster processes it. | ||
| if verr := tx.VerifySignatures(); verr != nil { | ||
| err = fmt.Errorf("%w: %w", ErrInvalidSignatures, verr) | ||
| return solana.Signature{}, err | ||
| } |
There was a problem hiding this comment.
Is this related to the sdk bump, or is it a gap that we always had? It looks correct to have this anyway, but I wonder if it's safe to add and what downstream effects it might have.
There was a problem hiding this comment.
yeah this gap has always been here, somehow got revealed with solana airdrop constantly failing after sdk bump
| result := <-job.blocks | ||
| require.Equal(t, slotNumber, result.SlotNumber) | ||
| require.Equal(t, &block.Blockhash, result.BlockHash) | ||
| require.Empty(t, result.Events) // no "Program log:"/"Program data:" lines in this tx, so no events expected |
There was a problem hiding this comment.
I'll try to look for (or craft) a v1 tx with some events and inner cross-program invocations, so we can test the event parsing and traversal of inner instructions too
make tidy fix integration tests add test case with v1 transaction fix comment chore: empty commit to retrigger CI
ca568c4 to
5040af6
Compare
|
3 participants
core ref: 1294f58cb0ddeed3535c12339b08dadd7cc2ce3b
Bump gagliardetto/solana-go v1.13.0 → v1.23.0
Why
Picks up upstream fixes/features (v1 transaction support, account state helpers, etc.) and lets us drop a local stopgap.
What had to change
client.go:MaxSupportTransactionVersionraised to1— solana-go can now decode v1 messages natively, so the oldMaxRequestedTransactionVersionworkaround (requesting a higher version than we could decode, to avoidgetBlockfailing with-32015) is no longer needed.logpoller: removed that stopgap (job_get_block.go, deletedjob_get_block_version_test.go, trimmed related metrics).client.go: solana-go'sTransaction.MarshalBinarynow silently zero-pads missing signatures instead of erroring, soSendTxexplicitly callsVerifySignatures()first and returns a newErrInvalidSignatures;classify_errors.goclassifies it asFatal.client.go: replacedcommonhttp.LimitedTransportwith a localsizeLimitedTransport— solana-go's JSON-RPC client now decodes withgoccy/go-json, whose stream reader swallows non-EOF errors from the response body, so the size-limit error from the old lazy (read-time) transport never surfaced. The new transport enforces the limit eagerly inRoundTrip, before decoding starts.chainwriter: fixedTestChainWriter_SubmitTransaction— solana-go'sAccountMeta.less()now tie-breaks same signer/writable-tier accounts by raw pubkey bytes instead of preserving insertion order, soNewTransaction's compiledAccountKeysare no longer in program-declared order. This doesn't affect execution correctness (each instruction's account references are still resolved by pubkey viaCompiledInstruction.Accounts), but the test asserted on fixed indices. Updated it to check account membership/order via the compiled instruction indices instead of rawAccountKeysposition.