Skip to content

clippy: Box result in RpcCustomError::SendTransactionPreflightFailure#11191

Closed
kskalski wants to merge 1 commit intoanza-xyz:masterfrom
kskalski:ks/clippy_large_err
Closed

clippy: Box result in RpcCustomError::SendTransactionPreflightFailure#11191
kskalski wants to merge 1 commit intoanza-xyz:masterfrom
kskalski:ks/clippy_large_err

Conversation

@kskalski
Copy link
Copy Markdown

@kskalski kskalski commented Mar 11, 2026

Problem

Clippy in Rust toolchain 1.94 reports clippy:result-large-err warning for large error in rpc code.

error: the `Err`-variant returned from this closure is very large
    --> rpc/src/rpc.rs:1384:45
     |
1384 |   ...                   .spawn_blocking(move || {
     |  _______________________________________^
1385 | | ...                       confirmed_block
1386 | | ...                           .encode_with_options(encoding, encoding_options)
1387 | | ...                           .map_err(RpcCustomError::from)
1388 | | ...                   })
     | |_______________________^ the `Err`-variant is at least 376 bytes
     |
     = help: try reducing the size of `solana_rpc_client_api::custom_error::RpcCustomError`, for example by boxing large elements or replacing it with `Box<solana_rpc_client_api::custom_error::RpcCustomError>`

Summary of Changes

Box result field in SendTransactionPreflightFailure variant

@mergify
Copy link
Copy Markdown

mergify Bot commented Mar 11, 2026

If this PR represents a change to the public RPC API:

  1. Make sure it includes a complementary update to rpc-client/ (example)
  2. Open a follow-up PR to update the JavaScript client @solana/kit (example)

Thank you for keeping the RPC clients in sync with the server API @kskalski.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.2%. Comparing base (9554223) to head (2db6977).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #11191   +/-   ##
=======================================
  Coverage    83.2%    83.2%           
=======================================
  Files         841      841           
  Lines      318322   318323    +1     
=======================================
+ Hits       265020   265027    +7     
+ Misses      53302    53296    -6     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kskalski kskalski marked this pull request as ready for review March 11, 2026 05:56
@kskalski kskalski requested a review from brooksprumo March 11, 2026 05:56
@brooksprumo
Copy link
Copy Markdown

Which errors? In the past I've boxed large errors that were within our own error enums to handle similar lints.

@kskalski kskalski force-pushed the ks/clippy_large_err branch from 10ce7c9 to f7701e4 Compare March 11, 2026 13:46
@kskalski
Copy link
Copy Markdown
Author

Looks like bigtable is no longer causing issues, but rpc does solana_rpc_client_api::custom_error::RpcCustomError

@kskalski kskalski changed the title clippy: allow large result errors in rpc and storage-bigtable clippy: allow large result errors in rpc Mar 11, 2026
@brooksprumo
Copy link
Copy Markdown

How about boxing the result field inside the SendTransactionPreflightFailure error variant? That type is RpcSimulateTransactionResult, which is 352 bytes.

@kskalski kskalski force-pushed the ks/clippy_large_err branch from f7701e4 to 2db6977 Compare March 11, 2026 14:41
@kskalski kskalski changed the title clippy: allow large result errors in rpc clippy: Box result in RpcCustomError::SendTransactionPreflightFailure Mar 11, 2026
@brooksprumo
Copy link
Copy Markdown

brooksprumo commented Mar 11, 2026

Ah right... this is a public-facing API.

https://docs.rs/solana-rpc-client-api/latest/solana_rpc_client_api/custom_error/enum.RpcCustomError.html#variant.SendTransactionPreflightFailure

But we haven't upgraded the crate since v3.1?

I think this might be a semver-breaking change then. Ugh.

Here's the first PR where I did something similar: #6290

And the PR where I did the boxing (which mentions the api-breaking-ness): #6293

Maybe the clippy-allow is the only way forward until the next major version.

@kskalski
Copy link
Copy Markdown
Author

But we haven't upgraded the crate since v3.1?

I think until 4.0 ships we use -beta labels, there are versions with that on https://docs.rs/solana-rpc-client-api/4.0.0-beta.1/solana_rpc_client_api/custom_error/enum.RpcCustomError.html

I guess the optimal course is to do this change and BP to 4.0. Otherwise, maybe need to go back to the previous way of just masking the lint in the call sites, which were not too many.

@brooksprumo
Copy link
Copy Markdown

May be worth asking in Discord on the correct way to handle this.

@kskalski
Copy link
Copy Markdown
Author

It was suggested on discord that we just allow the lint, since rpc is going to be removed anyway. I have it on another PR.

@kskalski kskalski closed this Mar 18, 2026
@kskalski kskalski deleted the ks/clippy_large_err branch March 18, 2026 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants