Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix scriptPubKey terminology #563

Closed
wants to merge 7 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

petertodd commented Sep 19, 2014

This is causing quite a bit of confusion, for example by people looking
in the Bitcoin Core sourcecode and seeing the term 'scriptPubKey'
instead.

Also fixed a few other errors from the transacitons reference section.

petertodd added some commits Sep 19, 2014

Fix OP_RETURN misconception
Satoshis sent to an OP_RETURN-using output are simply unspendable; they
do not go to mining fees.
Remove OP_1NEGATE mention
Never used in standard transactions
Replace misleading 'return' terminology with 'push'
Saying opcodes return values gives the impression the script is being
returned.
Fix incorrect usage of 'script' rather than 'scriptPubKey'
This is causing quite a bit of confusion, for example by people looking
in the Bitcoin Core sourcecode and seeing the term 'scriptPubKey'
instead.

@petertodd petertodd changed the title from Fix script pub key terminology to Fix scriptPubKey terminology Sep 19, 2014

Owner

petertodd replied Sep 19, 2014

There's a lot that needs to be fixed... I didn't realise how bad it all was until I told a client's developers to go read the dev docs to get up to speed and had them come back with tonnes of questions. :(

Why remove the true part (rendering the output unspendable)?

Owner

petertodd replied Sep 19, 2014

Because it's not necessarily true - OP_RETURN only renders the output unspendable if it's executed. Saying "rendering the output unspendable" is redundent and leads people to believe the presence of the opcode always does that; note how all the OP_VERIFY etc. descriptions simply say "terminate the script in failure"

Contributor

harding commented Sep 19, 2014

The OP_RETURN and OP_1NEGATE commits fix outright errors, so I'm going to cherry pick and push them in a few minutes.

The text changed in commit 1a8979b was based on a suggestion by @gmaxwell, so I'd appreciate feedback from him, if possible.

As for replacing "input script" and "output script" with the terms used in Bitcoin Core, I don't have any objection, although I do wish we had a term that was less confusing than scriptPubKey. If anyone doesn't want this changed, please comment---otherwise, I'll more throughly review this with the aim of merging it in a few days.

Contributor

petertodd commented Sep 19, 2014

re: scriptPubKey, the way I've been explaining it is to say that Bitcoin implements a generalization of a standard pubkey crypto system where rather than directly using pubkeys and signatures it indirectly uses pubkeys and signatures through a small forth-like language. This keeps it clear that the scripting system is fundementally about authorization and verification - just like any other pubkey crypto system - rather than some kind of "computation" being performed.

Secondly given that the Satoshi Bitcoin Core codebase is fairly simple I always strongly recommend that developers familiarize themselves with it directly.

re: 1a8979b open to debate - my thinking there is we don't need newer developers to worry about the history of why scriptSig and scriptPubKey were separated. What's more important is that whatever you put in a scriptSig that isn't data isn't signed anyway, so there is no point.

Contributor

harding commented Sep 19, 2014

@petertodd that's a good explanation. I may see if I can put that somewhere in the docs.

If your client's devs have time, we would really appreciate feedback on the docs. Bug reports are great, because there are mistakes, but I'd love to also get suggestions on how we can improve the guide so that they don't have so many questions after reading it. Devs can submit issues here, post to our mailing list, or email me directly at dave@dtrt.org.

Thanks again for the pull. I'll try to look at it in more detail tomorrow.

Contributor

saivann commented Sep 19, 2014

I concur, this kind of feedback would be highly useful.

Contributor

luke-jr commented Sep 19, 2014

scriptPubKey and scriptSig come from the hungarian notation BCCore used to use. Probably it would be ideal to just refer to "pubkeys" and "signatures" with an understanding both of those are scripts in Bitcoin. Maybe being more verbose with "pubkey script" and "signature script" avoids more questions though.

I had to read this a few times before I followed it (grammar, not logic). Maybe reword slightly?

Note: transactions can add non-data-pushing op codes to scriptSigs, but
because the CHECKSIG (and similar) opcodes strip the contents of scriptSigs this is not useful nor
secure. Such transactions are currently considered non-standard and in the future this
ability may be removed.
Owner

petertodd replied Sep 19, 2014

I think that's better. How about this?

Note: transactions can add non-data-pushing op codes to scriptSigs, but because the CHECKSIG (and
similar) opcodes remove the contents of the scriptSig prior to signing the transaction this is not useful 
nor secure. Such transactions are currently considered non-standard and in the future this ability may
be removed.

P2SH redemptions already don't allow non-data-pushing opcodes - might be worth a mention here.

@petertodd & @luke-jr: how about this for the note on non-data-op-codes:

Note: Signature scripts are not signed, so anyone can modify them. This means signature scripts should only contain data and data-pushing op codes which can't be modified without causing the pubkey script to fail. Placing non-data-pushing op codes in the signature script currently makes a transaction non-standard, and future protocol rules may forbid such transactions altogether. (Non-data-pushing op codes are already forbidden in signature scripts when spending a P2SH pubkey script.)

I rephrased it to try to make the advice more clear ("should only contain...") and also mention the P2SH spending rules.

Contributor

harding commented Sep 19, 2014

I like how @luke-jr's suggested terms "pubkey script" and "signature script" re-enforce @petertodd's description of Bitcoin containing a generalized public key cryptosystem. I think we could enhance that effect by explicitly specifying "secp256k1 public keys" and "secp256k1 signatures" whenever referring to parts of that specific cryptosystem. For example:

The signature script that spends a P2SH multisig output contains one or more secp256k1 signatures and the redeem script belonging to the output being spent.

Using those suggested terms should allow novice Bitcoin developers to immediately figure out what scriptPubKey and scriptSig mean when they first encounter them.

Although it's a very minor concern, this would create nice parallelism between the terms "pubkey script", "signature script", and "redeem script". (Instead of scriptPubKey, scriptSig, and redeemScript.)

How do people feel about me doing the following:

  1. Integrating into the text a form of @petertodd's description of Bitcoin containing a generalized public key cryptosystem.
  2. Introducing the terms "pubkey script" and "signature script", and then changing the docs terminology to use them throughout the text.
  3. Introducing secp256k1 earlier than we do now, and then changing the docs terminology to use "secp256k1 public key", "hashed secp256k pubkey" (for P2PKH), and "secp256k1 signature " throughout the text.

@harding harding and 1 other commented on an outdated diff Sep 19, 2014

_includes/guide_transactions.md
scriptSig: <sig> <pubkey>
~~~
{% autocrossref %}
-**Script Hash (P2SH)**
+**Pay 2 Script Hash (P2SH)**
@harding

harding Sep 19, 2014

Contributor

Recommend that we change this to "Pay To Script Hash (P2SH)".

@petertodd

petertodd Sep 20, 2014

Contributor

Agreed

Small fixes to scriptPubKey/scriptSig pull
* Small grammar fixes.

* Lower case #term-scriptPubKey as all our other anchor links are
  lower case

* Replace script/scripts with scriptPubKey/scriptPubKeys in
  _autocrossref.yaml. (Fixes `make test` errors from broken
  auto-crossref links.)
Contributor

harding commented Sep 20, 2014

I opened pull #566 which makes the changes described in my comment above---specifically, using the terms "pubkey script" and "signature script" suggested by @luke-jr.

That pull does not include any version of the rephrased paragraph about data-pushing opcodes in signature scripts. I figure we can work on that once we decide what terminology to use. All of @petertodd's other commits in this pull have already been committed or are included in pull #566. Thanks!

Merge pull request #1 from harding/script-terminology-edit
Small fixes to scriptPubKey/scriptSig pull
Contributor

saivann commented Oct 23, 2014

IIRC, This pull request is still open due to pending discussions on commit 1a8979b.

@harding If that sounds good to you, I suggest you merge your version for now (and at some point revisit the glossary idea to address inconsistent vocabulary as suggested by @petertodd)

@harding harding closed this in 1061571 Oct 23, 2014

Contributor

harding commented Oct 23, 2014

As suggested by @saivann, the following version of the revised paragraph was added to the text in commit 1061571:

Note: Signature scripts are not signed, so anyone can modify them. This
means signature scripts should only contain data and data-pushing op
codes which can't be modified without causing the pubkey script to fail.
Placing non-data-pushing op codes in the signature script currently
makes a transaction non-standard, and future consensus rules may forbid
such transactions altogether. (Non-data-pushing op codes are already
forbidden in signature scripts when spending a P2SH pubkey script.)

Problems or suggestions for improvements to that paragraph can be made by issue or pull request. Thanks!

jmaurice added a commit to jmaurice/bitcoin.jp that referenced this pull request Nov 19, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment