Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add archival version option to Bank RPC, README and test cases #1284

Merged
merged 4 commits into from
Jan 9, 2024

Conversation

dubbelosix
Copy link
Contributor

@dubbelosix dubbelosix commented Jan 8, 2024

Description

In order to make use of archival version support from the RPC level, we decided to explicitly add "version" as a parameter to any rpc query needing to make use of it.

diff illustrating the changes to add versioning to a specific RPC query

     pub fn balance_of(
         &self,
+        version: Option<u64>,
         user_address: C::Address,
         token_address: C::Address,
         working_set: &mut WorkingSet<C>,
     ) -> RpcResult<BalanceResponse> {
+        if let Some(v) = version {
+            working_set.set_archival_version(v)
+        }
         Ok(BalanceResponse {
             amount: self.get_balance_of(user_address, token_address, working_set),
         })

Changes

  • Modified bank module balance_of and supply_of
  • Modified RPC_WALKTHROUGH.md readme with instructions for archival support
  • Modified examples/demo-rollup/tests/bank/mod.rs to have an integration test for querying previous versions

@dubbelosix dubbelosix requested a review from bkolad January 8, 2024 13:30
@dubbelosix dubbelosix marked this pull request as ready for review January 8, 2024 13:30
Copy link
Member

@bkolad bkolad left a comment

Choose a reason for hiding this comment

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

LGTM, can you please update sov-rollup-starter as well after this PR is merged.

Copy link

codecov bot commented Jan 9, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (7d97267) 79.1% compared to head (db84955) 79.1%.

❗ Current head db84955 differs from pull request most recent head 4136827. Consider uploading reports for the commit 4136827 to get more accurate results

Additional details and impacted files
Files Coverage Δ
...ystem/module-implementations/sov-bank/src/query.rs 96.6% <87.5%> (-3.4%) ⬇️
module-system/sov-cli/src/workflows/rpc.rs 0.0% <0.0%> (ø)

... and 1 file with indirect coverage changes

@dubbelosix dubbelosix added this pull request to the merge queue Jan 9, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 9, 2024
@dubbelosix dubbelosix added this pull request to the merge queue Jan 9, 2024
@dubbelosix dubbelosix removed this pull request from the merge queue due to a manual request Jan 9, 2024
@dubbelosix dubbelosix added this pull request to the merge queue Jan 9, 2024
Merged via the queue into nightly with commit 5884991 Jan 9, 2024
17 checks passed
@dubbelosix dubbelosix deleted the dub/simple_archival_rpc branch January 9, 2024 15:32
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