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

fix(rpc): Return full network upgrade activation list in getblockchaininfo method #8699

Merged
merged 1 commit into from
Jul 20, 2024

Conversation

arya2
Copy link
Contributor

@arya2 arya2 commented Jul 18, 2024

Motivation

Lightwalletd relies on the Sapling activation height from the getblockchaininfo response to skip syncing pre-Sapling blocks.

Solution

  • Adds a full_activation_list() method on Network
  • Replaces the call to activation_list() in the get_block_chain_info RPC method with a call to full_activation_list()
  • Updates docs

Tests

Adds a vector test to check that full_activation_list() returns all implicit network upgrade activations

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

@arya2 arya2 added A-rpc Area: Remote Procedure Call interfaces lightwalletd any work associated with lightwalletd P-Medium ⚡ labels Jul 18, 2024
@arya2 arya2 requested a review from a team as a code owner July 18, 2024 17:26
@arya2 arya2 requested review from upbqdn and removed request for a team July 18, 2024 17:26
@arya2 arya2 self-assigned this Jul 18, 2024
@arya2 arya2 added the C-bug Category: This is a bug label Jul 18, 2024
@arya2 arya2 requested a review from oxarbitrage July 18, 2024 19:50
@oxarbitrage
Copy link
Contributor

I think the changes looks good but can we add more context? Do we have a lightwalletd issue or discussion we can link here ?

@mpguerra
Copy link
Contributor

Related to: https://discord.com/channels/809218587167293450/809251029673312267/1263134871505735680

Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

utACK

@oxarbitrage
Copy link
Contributor

Related to: https://discord.com/channels/809218587167293450/809251029673312267/1263134871505735680

Thanks, according to the discord discussion it seems the PR fixes the issue.

mergify bot added a commit that referenced this pull request Jul 19, 2024
@mergify mergify bot merged commit 6b9b994 into main Jul 20, 2024
135 checks passed
@mergify mergify bot deleted the full-activation-list branch July 20, 2024 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Area: Remote Procedure Call interfaces C-bug Category: This is a bug lightwalletd any work associated with lightwalletd P-Medium ⚡
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants