Skip to content

Conversation

@t-nelson
Copy link

@t-nelson t-nelson commented Oct 9, 2025

Problem

according to serde-rs/serde#2608, the issue below has been resolved upstream

agave/Cargo.toml

Lines 358 to 361 in f1fb3d2

serde = "1.0.228" # must match the serde_derive version, see https://github.com/serde-rs/serde/issues/2584#issuecomment-1685252251
serde-big-array = "0.5.1"
serde_bytes = "0.11.19"
serde_derive = "1.0.228" # must match the serde version, see https://github.com/serde-rs/serde/issues/2584#issuecomment-1685252251

Summary of Changes

replace serde_derive crate dep with the derive feature on the serde crate

no apparent degradation in build time on my workstation

Screenshot from 2025-10-09 17-01-38

@t-nelson t-nelson requested review from a team as code owners October 9, 2025 23:06
@mergify
Copy link

mergify bot commented Oct 9, 2025

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 @t-nelson.

@mergify
Copy link

mergify bot commented Oct 9, 2025

For your information, the solana-zk-token-sdk is deprecated, and this
directory will be removed in a future version. Please take this in mind
when making modifications.

@t-nelson t-nelson requested a review from a team October 9, 2025 23:09
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.2%. Comparing base (fbc8c81) to head (59bfa41).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8421   +/-   ##
=======================================
  Coverage    83.2%    83.2%           
=======================================
  Files         846      846           
  Lines      369242   369246    +4     
=======================================
+ Hits       307372   307448   +76     
+ Misses      61870    61798   -72     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@alexpyattaev alexpyattaev left a comment

Choose a reason for hiding this comment

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

LGTM on the gossip, votor, net-utils

Copy link

@steviez steviez left a comment

Choose a reason for hiding this comment

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

no apparent degradation in build time on my workstation

Feature unification means there should be not be any hit building the workspace if I'm not mistaken. Rather, I believe any build time degradation would show up in a crate that is using serde but not the serde_derive.

Either way, LGTM

Another win from this is these derive macros were the source of lots of false positives with tools like cargo machete IIRC; might have more reasonable output with this in

@t-nelson
Copy link
Author

Feature unification means there should be not be any hit building the workspace if I'm not mistaken. Rather, I believe any build time degradation would show up in a crate that is using serde but not the serde_derive.

nah read the original issue. bug was that the original derive feature on serde crate serialized serde_derive crate build with everything that depends on it. workaround was to not use the crate feature, but instead directly depend on serde_derive. recently, the broke out the common parts to serde_core allowing everything to proceed in parallel

@t-nelson t-nelson merged commit 349f080 into anza-xyz:master Oct 10, 2025
54 checks passed
@t-nelson t-nelson deleted the rmsd branch October 10, 2025 16:13
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.

6 participants