Skip to content

BIP 141: Segregated Witness (Consensus layer)#265

Merged
luke-jr merged 4 commits intobitcoin:masterfrom
CodeShark:segwit
Jan 8, 2016
Merged

BIP 141: Segregated Witness (Consensus layer)#265
luke-jr merged 4 commits intobitcoin:masterfrom
CodeShark:segwit

Conversation

@CodeShark
Copy link
Copy Markdown
Contributor

BIP number requested,

@CodeShark CodeShark force-pushed the segwit branch 5 times, most recently from 78eb898 to 7f19639 Compare December 23, 2015 01:54
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missing authors

@CodeShark CodeShark force-pushed the segwit branch 4 times, most recently from faf8ab2 to ec059d1 Compare December 23, 2015 02:23
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

???

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the comment refers to the p2p serialization and relay stuff - what follows is the consensus specification, which WILL be in this document.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This was confusing to me. You're not talking about Version 0 and Version 1 witness programs here are you? Aren't you just talking about whether or not the witness program is also embedded in a P2SH regardless of what version witness it is?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah, the description is somewhat inaccurate. I'll edit it

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not a single push anymore

Clarify Backward compatibility and separating version byte and witness program
@rubensayshi
Copy link
Copy Markdown
Contributor

related BIPs, for easy reference;

and WIP code:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The segwit update seems like the perfect moment to get rid of that CHECKMULTISIG bug where one item too many is popped off the stack. Although probably should be done in #270.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No bikeshedding about what changes to make, please :(

We have script versions now, so we do not need to fix all inconveniences at
once.

There are two exceptions to this self-imposed rule:

  • Changing the sighashing to not be O(n^2), as it wouldn't be a bugfix to a
    worst-case scenario if it's optional for an attacker.
  • Amount signing, as I didn't expect that people would accept any sighash
    change proposal that did not include this...

@rubensayshi
Copy link
Copy Markdown
Contributor

is there a reason why the transaction version is not bumped for the new structure?

@sipa
Copy link
Copy Markdown
Member

sipa commented Jan 7, 2016

Yes. There is no need:

  • SegWit transactions are not standard anyway due to the CLEANSTACK rule
    already.
  • We don't need it for serialization, because we need another mechanism for
    signaling the presence of witness data anyway.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: use OP_0 <DUP HASH160 {20-byte-hash-value} EQUALVERIFY CHECKSIG>?

Copy link
Copy Markdown
Member

@sipa sipa Jan 7, 2016 via email

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

@sipa sipa Jan 7, 2016 via email

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@jonasschnelli Isn't the deserialized format below?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's shown in the deserialized script

@luke-jr luke-jr self-assigned this Jan 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.