Skip to content

fix: use the config arg in the get_balance rpc method#547

Merged
MicaiahReid merged 6 commits intosolana-foundation:mainfrom
0xzrf:config_use_in_get_bal
Mar 10, 2026
Merged

fix: use the config arg in the get_balance rpc method#547
MicaiahReid merged 6 commits intosolana-foundation:mainfrom
0xzrf:config_use_in_get_bal

Conversation

@0xzrf
Copy link
Contributor

@0xzrf 0xzrf commented Feb 28, 2026

Summary

This PR refactors SurfpoolMinimalRpc::get_balance() to use the config: Option<RpcContextConfig> argument, which provides the commitment: Option<CommitmentConfig> and min_context_slot: Option<u64> variables, in order to customize the get_balance rpc method further

What Changed

  • Extracted commitment and min_context_slot variables, and provided Commitment::confirmed()(the default earilier) to commitment and None to min_context_slot in case None
  • use the commitment provided in config inside the meta.get_rpc_context() in case present, or Commitment::confirmed() otherwise
  • Use the min_context_slot value (if present) to validate the svm_locker.get_account().slot
  • Added a SurfpoolError::get_balance() method that returns a SurfpoolError::internal_error() with error.data = "Context slot too old"

Context

This change was motivated by an existing comment inside SurfpoolMinimalRpc::get_balance() that suggested using the config in the function. This PR increases the configurability of the rpc's get_balance method by allowing to provide the commitment level and min_context_slot as arguments

Note

I may or may not have used the min_context_slot value properly, as I've assumed that it's used to validate the value svm_locker.get_account().slot and require it to be bigger then then the argument. I'd be happy to change it's use case in case my assumption is wrong. Thanks

Copy link
Collaborator

@MicaiahReid MicaiahReid left a comment

Choose a reason for hiding this comment

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

Thanks for the issue! Some small fixes requested.

@0xzrf
Copy link
Contributor Author

0xzrf commented Mar 2, 2026

@MicaiahReid I've made the changes
lmk if you have any feedbacks. Thanks

Copy link
Collaborator

@MicaiahReid MicaiahReid left a comment

Choose a reason for hiding this comment

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

Sorry, should have requested earlier, can we also add some tests for these changes?

Comment on lines +106 to +110
pub fn get_balance() -> Self {
let mut error = Error::internal_error();
error.data = Some(json!("Context slot too old"));
Self(error)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we remove this error that is no longer being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it

@MicaiahReid MicaiahReid requested a review from lgalabru March 3, 2026 16:19
@0xzrf
Copy link
Contributor Author

0xzrf commented Mar 5, 2026

@MicaiahReid added some tests that check whether the get_balance rpc method fail when the min_context_slot is bigger then latest_absolute_slot()

lmk if anything else is required. Thanks

@0xzrf
Copy link
Contributor Author

0xzrf commented Mar 7, 2026

@MicaiahReid Added config support for the SurfpoolFullRpc::get_fee_for_message in the last commit, ik the PR specifically fixes the get_balance, but theoratically the fixes for both the rpc methods felt the same.

Copy link
Collaborator

@MicaiahReid MicaiahReid left a comment

Choose a reason for hiding this comment

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

Almost there!

@MicaiahReid MicaiahReid merged commit f7386ab into solana-foundation:main Mar 10, 2026
5 checks passed
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.

2 participants