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

feat(rpc): added optional block height in getassetunlockstatuses #5849

Conversation

ogabrielides
Copy link
Collaborator

@ogabrielides ogabrielides commented Jan 28, 2024

Issue being fixed or feature implemented

RPC getassetunlockstatuses is now accepting an extra optional parameter height.
When a valid height is passed, then the RPC returns the status of AssetUnlock indexes up to this specific block. (Requested by Platform team)

What was done?

Note that in order to avoid cases that can lead to deterministic result, when height is passed, then the only chainlocked and unknown outcomes are possible.

How Has This Been Tested?

feature_asset_locks.py was updated.

Breaking Changes

n/a

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@ogabrielides ogabrielides modified the milestones: 21, 20.1 Jan 28, 2024
@ogabrielides ogabrielides added the RPC Some notable changes to RPC params/behaviour/descriptions label Jan 28, 2024
@DashCoreAutoGuix
Copy link

Guix Automation has failed due to the HEAD commit not being signed by an authorized core-team member. Please rebase and sign or push a new empty signed commit to allow Guix build to happen.

@knst knst force-pushed the rpc_getassetunlockstatuses_chainlock_fix branch from a6d92bf to c597fb4 Compare January 28, 2024 20:59
@DashCoreAutoGuix
Copy link

Guix Automation has began to build this PR tagged as v20.1.0-devpr5849.c597fb4d. A new comment will be made when the image is pushed.

@knst knst removed the guix-build label Jan 28, 2024
@knst
Copy link
Collaborator

knst commented Jan 28, 2024

@knst knst force-pushed the rpc_getassetunlockstatuses_chainlock_fix branch from a6d92bf to c597fb4

signed commit to get guix build

@DashCoreAutoGuix
Copy link

Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v20.1.0-devpr5849.c597fb4d. The image should be on dockerhub soon.

src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved
@ogabrielides ogabrielides marked this pull request as draft January 29, 2024 11:04
@ogabrielides ogabrielides marked this pull request as ready for review January 29, 2024 13:47
@ogabrielides ogabrielides changed the title fix(rpc): correct handling of CL info in getassetunlockstatuses. feat(rpc): added optional block height in getassetunlockstatuses Jan 29, 2024
@DashCoreAutoGuix
Copy link

Guix Automation has failed due to the HEAD commit not being signed by an authorized core-team member. Please rebase and sign or push a new empty signed commit to allow Guix build to happen.

@DashCoreAutoGuix
Copy link

Guix Automation has began to build this PR tagged as v20.1.0-devpr5849.d7408624. A new comment will be made when the image is pushed.

@knst knst removed the guix-build label Jan 29, 2024
@DashCoreAutoGuix
Copy link

Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v20.1.0-devpr5849.d7408624. The image should be on dockerhub soon.

@DashCoreAutoGuix
Copy link

Guix Automation has began to build this PR tagged as v20.1.0-devpr5849.7bfa0d7e. A new comment will be made when the image is pushed.

@DashCoreAutoGuix
Copy link

Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v20.1.0-devpr5849.7bfa0d7e. The image should be on dockerhub soon.

Copy link
Collaborator

@thephez thephez left a comment

Choose a reason for hiding this comment

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

Some small suggestions on wording

src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved
doc/release-notes-5776.md Outdated Show resolved Hide resolved
doc/release-notes-5776.md Outdated Show resolved Hide resolved
@ogabrielides ogabrielides requested a review from thephez January 29, 2024 19:58
src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved
UdjinM6
UdjinM6 previously approved these changes Jan 30, 2024
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

knst
knst previously approved these changes Jan 31, 2024
Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

utACK

if you are going to push more changes - consider my nits

src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved
src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved
@ogabrielides ogabrielides dismissed stale reviews from knst and UdjinM6 via 0d391eb January 31, 2024 22:48
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

re-utACK

Copy link
Collaborator

@thephez thephez left a comment

Choose a reason for hiding this comment

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

utACK

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK for squash merge

@PastaPastaPasta PastaPastaPasta merged commit 82310b0 into dashpay:develop Feb 1, 2024
9 of 11 checks passed
@ogabrielides ogabrielides deleted the rpc_getassetunlockstatuses_chainlock_fix branch February 2, 2024 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RPC Some notable changes to RPC params/behaviour/descriptions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants