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

BIP112: Update document to match implementation #248

Merged
merged 5 commits into from Nov 28, 2015

Conversation

btcdrak
Copy link
Contributor

@btcdrak btcdrak commented Nov 23, 2015

@NicolasDorier
Copy link
Contributor

Not good, remove:

Otherwise, script execution will continue as if a NOP had been executed.

This is not true, if one of those conditions are true, then script fail. If none of them are true, then processing will continue as defined in this BIP.

The only condition this is considered a NOP is if SCRIPT_VERIFY_CHECKSEQUENCEVERIFY flag is disabled. Which is not worth mentioning in the BIP.

@btcdrak
Copy link
Contributor Author

btcdrak commented Nov 25, 2015

@NicolasDorier This is the same language used in BIP65. If the script does not fail, script execution continues as if a NOP had been executed.

@NicolasDorier
Copy link
Contributor

Ok I see. Well, it is strange to say "executed as a NOP" as opposed to "the stack is not affected by this operation".
"Executed as NOP" for me mean no verification for failure is even made.

It does not stop me from understanding just finding it weird. Not a stopper though, if nobody complains let's keep it like that.

Taken from 20/11/15 version of deployable lightning
@NicolasDorier
Copy link
Contributor

ACK.

I would replace "Otherwise, script execution will continue as if a NOP had been executed." with "OP_CSV has no effect on the stack". But not worth complaining too much about if I am the only one to think so.

luke-jr added a commit that referenced this pull request Nov 28, 2015
BIP112: Update document to match implementation
@luke-jr luke-jr merged commit dd20802 into bitcoin:master Nov 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants