Skip to content

scrub use of ConfigState from config program client#5963

Merged
mergify[bot] merged 5 commits intoanza-xyz:masterfrom
buffalojoec:drop-config-state
Jun 6, 2025
Merged

scrub use of ConfigState from config program client#5963
mergify[bot] merged 5 commits intoanza-xyz:masterfrom
buffalojoec:drop-config-state

Conversation

@buffalojoec
Copy link
Copy Markdown

Problem

We're trying to get rid of the ConfigState trait, since it's not super useful. It's also not necessary in Agave at all.

Summary of Changes

Scrub all use of ConfigState, and use the helpers from the config client that only require serde::Serialize.
Note this depends on the linked branch for now, so it's to remain a draft until that lands.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 24, 2025

Codecov Report

Attention: Patch coverage is 4.16667% with 23 lines in your changes missing coverage. Please review.

Project coverage is 82.8%. Comparing base (cedfdf8) to head (29ac55e).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5963   +/-   ##
=======================================
  Coverage    82.8%    82.8%           
=======================================
  Files         848      847    -1     
  Lines      379490   379488    -2     
=======================================
  Hits       314482   314482           
+ Misses      65008    65006    -2     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@buffalojoec buffalojoec requested a review from joncinque April 25, 2025 00:59
@buffalojoec buffalojoec marked this pull request as ready for review April 25, 2025 00:59
Copy link
Copy Markdown

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks good to me! Just a tiny nit.

Once the new config program client crate is available, and the PR is updated to use it, I'll approve.

Comment thread install/src/command.rs
Comment thread account-decoder/src/validator_info.rs
@buffalojoec
Copy link
Copy Markdown
Author

I don't think CI is me...

@joncinque
Copy link
Copy Markdown

Hm, looks like it's picking up a newer version of solana-native-token, which is causing those warnings to throw. I put in #6428 to silence the warnings

Copy link
Copy Markdown

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Just a few last things to update, then this is good to go!

Comment thread cli/Cargo.toml Outdated
Comment thread install/Cargo.toml Outdated
Copy link
Copy Markdown

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks good to me! Will just need a rebase once the CI fix lands

@buffalojoec buffalojoec added the automerge automerge Merge this Pull Request automatically once CI passes label Jun 6, 2025
@mergify mergify Bot merged commit c163a92 into anza-xyz:master Jun 6, 2025
59 checks passed
@buffalojoec buffalojoec deleted the drop-config-state branch June 6, 2025 02:48
mircea-c pushed a commit to mircea-c/agave that referenced this pull request Jun 12, 2025
* scrub use of `ConfigState` from config program client

* review feedback

* use updated interface and client libs

* Update Cargo.toml

Co-authored-by: Jon C <me@jonc.dev>

* Update Cargo.toml

Co-authored-by: Jon C <me@jonc.dev>

---------

Co-authored-by: Jon C <me@jonc.dev>
mircea-c added a commit to mircea-c/agave that referenced this pull request Jun 12, 2025
* scrub use of `ConfigState` from config program client

* review feedback

* use updated interface and client libs

* Update Cargo.toml

Co-authored-by: Jon C <me@jonc.dev>

* Update Cargo.toml

Co-authored-by: Jon C <me@jonc.dev>

---------

Co-authored-by: Jon C <me@jonc.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge automerge Merge this Pull Request automatically once CI passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants