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: off-by-one in the way we use v19 activation helpers #5431

Merged
merged 2 commits into from
Jun 13, 2023

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Jun 12, 2023

Issue being fixed or feature implemented

Some conditions won't trigger when reorging exactly from the forkpoint

What was done?

pls see individual commits, tl;dr: you can't get correct results with GetAncestor cause the answer is in the future

How Has This Been Tested?

reorg to 850000 and back on testnet

invalidateblock 0000003eddb94218e7a3f41b2ac6e26143f8a748b50cd26e86bdbbab9ebe50aa
reconsiderblock 0000003eddb94218e7a3f41b2ac6e26143f8a748b50cd26e86bdbbab9ebe50aa

this fails on develop and work with this patch

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)

we no longer repopulate internal maps so this is no longer needed either
we need to know v19 activation height before the block is mined, GetAncestor is useless for this
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

@UdjinM6 UdjinM6 merged commit aa91946 into dashpay:develop Jun 13, 2023
@UdjinM6 UdjinM6 deleted the fix_off_by_1_v19_act branch June 13, 2023 14:24
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 13, 2023
Some conditions won't trigger when reorging exactly from the forkpoint

pls see individual commits, tl;dr: you can't get correct results with
`GetAncestor` cause the answer is in the future

reorg to 850000 and back on testnet
```
invalidateblock 0000003eddb94218e7a3f41b2ac6e26143f8a748b50cd26e86bdbbab9ebe50aa
reconsiderblock 0000003eddb94218e7a3f41b2ac6e26143f8a748b50cd26e86bdbbab9ebe50aa
```
this fails on develop and work with this patch

n/a

- [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)_
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.

post-merging ACK

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.

3 participants