Skip to content

BIP 174: clarify format of proprietary extensions.#951

Closed
rustyrussell wants to merge 1 commit intobitcoin:masterfrom
rustyrussell:clarify-178-proprietary
Closed

BIP 174: clarify format of proprietary extensions.#951
rustyrussell wants to merge 1 commit intobitcoin:masterfrom
rustyrussell:clarify-178-proprietary

Conversation

@rustyrussell
Copy link
Copy Markdown
Contributor

"Variable length string identifier" is not defined anywhere, and the suggestion
to use "0x00" is also deeply unclear. I assumed it meant a nul-terminated
string!

Be explicit: you mean it must be a compact siz1\e unsigned int length, followed
by that many identifier bytes, followed by a compact size unsigned int subtype,
followed by optional keydata.

Signed-off-by: Rusty Russell rusty@rustcorp.com.au

"Variable length string identifier" is not defined anywhere, and the suggestion
to use "0x00" is also deeply unclear.  I assumed it meant a nul-terminated
string!

Be explicit: you mean it must be a compact siz1\e unsigned int length, followed
by that many identifier bytes, followed by a compact size unsigned int subtype,
followed by optional keydata.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
** Key: Variable length identifier prefix, followed by a subtype, followed by the key data itself.
*** <tt>{0xFC}|<prefix>|{subtype}|{key data}</tt>
** Key: Compact size unsigned integer, followed by identifier prefix of that length, followed by a subtype, followed by the key data itself.
*** <tt>{0xFC}|{prefixlen}|<prefix>|{subtype}|{key data}</tt>
Copy link
Copy Markdown
Member

@achow101 achow101 Jul 28, 2020

Choose a reason for hiding this comment

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

The note on notation found earlier in the BIP indicates that <...> means "prefixed by compact size unsigned integer". So this is technically incorrect.

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.

Thanks, I missed that!

Is this boutique notation used in other BIPs? I haven't seen it before and it was completely non-intuitive to me!

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.

Is this boutique notation used in other BIPs? I haven't seen it before and it was completely non-intuitive to me!

I think I made up the notation as a placeholder and just never changed it to something else.

* Type: Proprietary Use Type <tt>PSBT_GLOBAL_PROPRIETARY = 0xFC</tt>
** Key: Variable length identifier prefix, followed by a subtype, followed by the key data itself.
*** <tt>{0xFC}|<prefix>|{subtype}|{key data}</tt>
** Key: Compact size unsigned integer, followed by identifier prefix of that length, followed by a subtype, followed by the key data itself.
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.

Because everything variable length in Bitcoin is prefixed by a compact size unsigned int, "Variable length ..." is the correct way to describe this. Further clarification of this should be in a parenthetical rather than listed as its own separate entity.

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.

This isn't even true in this BIP.

The key data is variable length. It is not prefixed by a compact size unsigned int!

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.

The key data is variable length. It is not prefixed by a compact size unsigned int!

Uhh... no? It definitely is prefixed by the size, it's just usually 0x01.

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.

Not quite: the entire key is prefixed by the size. The keydata length is implied.

You also use 0 as a terminator in various places in this BIP, eschewing explicit length there.

And bitcoin uses NUL as a terminator on command names in the wire protocol (within a fixed length field, because reasons?)

Given this lack of consistency, I think we're best off being horribly explicit?

@rustyrussell
Copy link
Copy Markdown
Contributor Author

Closing in favor of more ambitious approach....

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.

2 participants