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.

Sign up for GitHub

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

Jump to bottom

[Core] Fix Exchange address activation #2908

Merged
merged 2 commits into from
Feb 20, 2024
Merged

[Core] Fix Exchange address activation #2908

merged 2 commits into from
Feb 20, 2024

Conversation

Duddino
Copy link
Member

@Duddino Duddino commented Feb 19, 2024 •
edited
Loading

Change exchange address activation with the following:

  • Before 5.6 activation, accept TxOuts with OP_EXCHANGEADDR, even if they have sapling data in it
  • Before 5.6 activation, treat OP_EXCHANGEADDR as OP_UNKNOWN, thus rejecting any attempt at spending them
  • After 5.6 activation, keep things as is (Don't accept exchange address UTXOs if the tx has sapling data)

@Duddino Duddino force-pushed the exchange_activation branch 8 times, most recently from ed26983 to 84222dd Compare February 20, 2024 04:42
@Fuzzbawls Fuzzbawls added this to the 5.6.0 milestone Feb 20, 2024
@Fuzzbawls Fuzzbawls changed the title Fix Exchange address activation [Core] Fix Exchange address activation Feb 20, 2024
panleone
panleone requested changes Feb 20, 2024
View reviewed changes
Copy link

@panleone panleone left a comment •
edited
Loading

Choose a reason for hiding this comment

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

left a few comments, I would like to see test case in which someone try to spend after v5.6 activation a pre v5.6 exchange address tx and it fails

src/consensus/tx_verify.cpp Outdated Show resolved Hide resolved
src/script/interpreter.cpp Outdated Show resolved Hide resolved
src/script/interpreter.h Outdated Show resolved Hide resolved
src/script/sign.cpp Outdated Show resolved Hide resolved
src/script/sign.cpp Outdated Show resolved Hide resolved
src/test/transaction_tests.cpp Outdated Show resolved Hide resolved
src/validation.cpp Outdated Show resolved Hide resolved
src/validation.cpp Outdated Show resolved Hide resolved
test/functional/feature_exchangeaddr.py Outdated Show resolved Hide resolved
@Duddino
Copy link
Member Author

Duddino commented Feb 20, 2024 •
edited
Loading

I would like to see test case in which someone try to spend after v5.6 activation a pre v5.6 exchange address tx and it fails

This is already covered, test tries to spend OP_EXCHANGE_ADDR OP_1 and asserts that it fails
Sorry I misread that. Why should it fail?

Liquid369
Liquid369 approved these changes Feb 20, 2024
View reviewed changes
Copy link
Member

@Liquid369 Liquid369 left a comment

Choose a reason for hiding this comment

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

tACK d7d30dc

panleone
panleone approved these changes Feb 20, 2024
View reviewed changes
Copy link

@panleone panleone left a comment

Choose a reason for hiding this comment

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

utACK d7d30dc

@Fuzzbawls Fuzzbawls merged commit 8c359dd into PIVX-Project:master Feb 20, 2024
20 of 21 checks passed
Fuzzbawls pushed a commit that referenced this pull request Feb 20, 2024
@Duddino @Fuzzbawls
Fix Exchange address activation
0a47505 
Github-Pull: #2908
Rebased-From: 6c01283
Fuzzbawls pushed a commit that referenced this pull request Feb 20, 2024
@Duddino @Fuzzbawls
Change exchange address functional tests
1e97313 
Github-Pull: #2908
Rebased-From: d7d30dc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@panleone panleone panleone approved these changes

@Liquid369 Liquid369 Liquid369 approved these changes

Assignees

@Duddino Duddino

Labels
Bug Consensus
Projects
None yet
Milestone
5.6.0
Development

Successfully merging this pull request may close these issues.
4 participants
@Duddino @panleone @Liquid369 @Fuzzbawls