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): pass blockhash into TxToJSON so that getspecialtxes could show correct instantlock/chainlock values #5774

Merged
merged 3 commits into from
Dec 19, 2023

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Dec 18, 2023

Issue being fixed or feature implemented

instantlock and chainlock are broken in getspecialtxes

kudos to @thephez for finding the issue

What was done?

pass the hash and also rename the variable to self-describing

How Has This Been Tested?

run getspecialtxes on a node with and without the patch

Breaking Changes

instantlock and chainlock will show actual values and not just false all the time now (not sure if that qualifies for "breaking" though)

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)

@UdjinM6 UdjinM6 added the RPC Some notable changes to RPC params/behaviour/descriptions label Dec 18, 2023
@PastaPastaPasta
Copy link
Member

I think that a bug fix like this to match expected behavior; in a rpc that is rarely used is okay to be included in a minor / patch; assuming that we do document it as it is technically a breaking change. But like I said don't think anyone could possibly be relying on this behavior

@shumkov please object now or forever hold your peace

However @UdjinM6 can we improve tests at all? this probably should've been caught by tests no?

Copy link
Collaborator

@ogabrielides ogabrielides left a comment

Choose a reason for hiding this comment

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

utACK. (I agree with @PastaPastaPasta this is not really a breaking change)

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 added this to the 20.1 milestone Dec 19, 2023
@PastaPastaPasta PastaPastaPasta merged commit 9a99a4a into dashpay:develop Dec 19, 2023
8 of 9 checks passed
ogabrielides pushed a commit to ogabrielides/dash that referenced this pull request Dec 22, 2023
…ld show correct `instantlock`/`chainlock` values (dashpay#5774)

## Issue being fixed or feature implemented
`instantlock` and `chainlock` are broken in `getspecialtxes`

kudos to @thephez for finding the issue

## What was done?
pass the hash and also rename the variable to self-describing

## How Has This Been Tested?
run `getspecialtxes` on a node with and without the patch

## Breaking Changes
`instantlock` and `chainlock` will show actual values and not just
`false` all the time now (not sure if that qualifies for "breaking"
though)

## Checklist:
- [x] 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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
@UdjinM6 UdjinM6 modified the milestones: 20.1, 20.0.3 Dec 24, 2023
PastaPastaPasta pushed a commit to ogabrielides/dash that referenced this pull request Dec 24, 2023
…ld show correct `instantlock`/`chainlock` values (dashpay#5774)

## Issue being fixed or feature implemented
`instantlock` and `chainlock` are broken in `getspecialtxes`

kudos to @thephez for finding the issue

## What was done?
pass the hash and also rename the variable to self-describing

## How Has This Been Tested?
run `getspecialtxes` on a node with and without the patch

## Breaking Changes
`instantlock` and `chainlock` will show actual values and not just
`false` all the time now (not sure if that qualifies for "breaking"
though)

## Checklist:
- [x] 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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
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.

3 participants