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

Implement NULLDUMMY softfork (BIP147) #8636

Merged
merged 1 commit into from Sep 22, 2016

Conversation

Projects
None yet
@jl2012
Member

jl2012 commented Aug 31, 2016

Alternative to #8533

This will enforce SCRIPT_VERIFY_NULLDUMMY on all segwit and non-segwit transactions when segwit is activated with BIP9.

As we may need more time to implement LOW_S softfork in a better way (see #8533 (comment)), the alternative plan is to implement only NULLDUMMY softfork in 0.13.1 and leave LOW_S as a policy at this moment.

@jl2012

This comment has been minimized.

Show comment
Hide comment
@jl2012

jl2012 Sep 2, 2016

Member

Need a 0.13.1 tag

Member

jl2012 commented Sep 2, 2016

Need a 0.13.1 tag

@fanquake fanquake added this to the 0.13.1 milestone Sep 2, 2016

@petertodd

This comment has been minimized.

Show comment
Hide comment
@petertodd

petertodd Sep 2, 2016

Contributor

Concept ACK

Contributor

petertodd commented Sep 2, 2016

Concept ACK

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Sep 2, 2016

Contributor

concept ACK

Contributor

dcousens commented Sep 2, 2016

concept ACK

@btcdrak

This comment has been minimized.

Show comment
Hide comment
@btcdrak

btcdrak Sep 2, 2016

Member

Concept ACK

Member

btcdrak commented Sep 2, 2016

Concept ACK

@btcdrak

This comment has been minimized.

Show comment
Hide comment
@btcdrak

btcdrak Sep 4, 2016

Member

needs backport tag.

Member

btcdrak commented Sep 4, 2016

needs backport tag.

@jl2012 jl2012 changed the title from Implement NULLDUMMY softfork to Implement NULLDUMMY softfork (BIP147) Sep 4, 2016

@jl2012

This comment has been minimized.

Show comment
Hide comment
@jl2012

jl2012 Sep 4, 2016

Member

This is an implementation of BIP147, which could be found at https://github.com/bitcoin/bips/blob/36496946860d71d4460437572e6c4c780b125cad/bip-0147.mediawiki

I propose that reviewers should explicitly acknowledge that this pull request is a correct implementation of a specified version of BIP147. So they should review the relevant codes, for example, in interpreter.cpp.

This is needed because in #8533 we just focused on the PR, but not the consensus codes being activated. NULLDUMMY should be reasonably simple for everything to do more careful review.

For example, a reviewer may use: utACK 482f852, ACK BIP147 3649694

Member

jl2012 commented Sep 4, 2016

This is an implementation of BIP147, which could be found at https://github.com/bitcoin/bips/blob/36496946860d71d4460437572e6c4c780b125cad/bip-0147.mediawiki

I propose that reviewers should explicitly acknowledge that this pull request is a correct implementation of a specified version of BIP147. So they should review the relevant codes, for example, in interpreter.cpp.

This is needed because in #8533 we just focused on the PR, but not the consensus codes being activated. NULLDUMMY should be reasonably simple for everything to do more careful review.

For example, a reviewer may use: utACK 482f852, ACK BIP147 3649694

@NicolasDorier

This comment has been minimized.

Show comment
Hide comment
@NicolasDorier
Member

NicolasDorier commented Sep 7, 2016

utACK 482f852

@btcdrak

This comment has been minimized.

Show comment
Hide comment
@btcdrak

btcdrak Sep 9, 2016

Member

utACK 482f852

This has been active on testnet3 for a few weeks already. Already tACK from previous PR #8533 (comment)

Member

btcdrak commented Sep 9, 2016

utACK 482f852

This has been active on testnet3 for a few weeks already. Already tACK from previous PR #8533 (comment)

@jameshilliard

This comment has been minimized.

Show comment
Hide comment
@jameshilliard

jameshilliard Sep 12, 2016

Contributor

utACK 482f852

Contributor

jameshilliard commented Sep 12, 2016

utACK 482f852

@rubensayshi

This comment has been minimized.

Show comment
Hide comment
@rubensayshi

rubensayshi Sep 13, 2016

Contributor

utACK 482f852

Contributor

rubensayshi commented Sep 13, 2016

utACK 482f852

@instagibbs

This comment has been minimized.

Show comment
Hide comment
@instagibbs

instagibbs Sep 19, 2016

Member

code review/utACK 482f852

Member

instagibbs commented Sep 19, 2016

code review/utACK 482f852

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Sep 22, 2016

Member

utACK 482f852

Member

laanwj commented Sep 22, 2016

utACK 482f852

@laanwj laanwj merged commit 482f852 into bitcoin:master Sep 22, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Sep 22, 2016

Merge #8636: Implement NULLDUMMY softfork (BIP147)
482f852 Implement NULLDUMMY softfork (Johnson Lau)

laanwj added a commit to laanwj/bitcoin that referenced this pull request Sep 26, 2016

Implement NULLDUMMY softfork
Github-Pull: #8636
Rebased-From: 482f852
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Sep 26, 2016

Member

This is backported in #8815, removing tag

Member

laanwj commented Sep 26, 2016

This is backported in #8815, removing tag

@laanwj laanwj removed the Needs backport label Sep 26, 2016

codablock added a commit to codablock/dash that referenced this pull request Jan 30, 2018

Merge #8636: Implement NULLDUMMY softfork (BIP147)
482f852 Implement NULLDUMMY softfork (Johnson Lau)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment