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

Add Developer Guide To Bitcoin.org #393

Merged
merged 28 commits into from May 24, 2014

Conversation

Projects
None yet
6 participants
Contributor

harding commented May 11, 2014

This pull request proposes to add to Bitcoin.org a Developer Guide providing the equivalent of 70 printed pages of introductions to key Bitcoin concepts.

A preview is available at: http://bitcoindev.us.to/en/developer-documentation

The Guide is not complete---there is much more we plan to write over the coming months---but the version in this pull request is internally consistent and reasonably comprehensive. In addition to the Guide, this pull request also includes a Developer Reference with 90 more pages of detailed technical material, such as RPC descriptions, and a portal page linking to the various sections of both the Guide and the Reference.

To keep comments on this pull request focused, we'd appreciate it if suggestions about possible future content or better ways to organize this information were made on our Content Suggestions Wiki Page, restricting comments on this pull request to only things which should be fixed or changed before merging.

If you want to help review but don't have time to read over 150 pages of text, we recommend that you review just one of the sections listed below, preferably one that hasn't been well-reviewed yet. We'll add your GitHub @ name to the Being Reviewed By column for any section where you leave a line comment, and add your name to the ACK column if you ACK any section.

Section Preview (Source Files) Being Reviewed By ACKs From
Block Chain (BC)
Transactions (T) @TierNolan
Contracts (C) @mikehearn
Wallets (W)
Payment Processing (PP)
Operating Modes (OM)
P2P Network (P2P)
Mining (M) @luke-jr
Block Chain & Transaction Reference (BC & T)
Bitcoin Core RPCs (A-G, H-N, O-T, U-Z)

If you leave a line comment or general comment for a particular section, we'd appreciate it if you ACK the section once it meets your expectations. We hope this will allow us to build consensus to merge without everyone having to read the entire text.

Many thanks are owed for the two months of work which went into this pull request. An incomplete, alphabetical list is: @cbeams, @harding, @instagibbs, @mikehearn, @saivann, and @tgeller.

TODO:

  • Replace P2PH by P2PKH. Fixed in #400 (merged)
  • Replace DOS by DoS. Fixed in #394 (merged)
  • GBT can use a persistent TCP socket, without a persistent session. Fixed in #394 (merged)
  • More emphasis on key reuse security risks? Fixed in #395 (merged)
  • Contracts section improvements suggested by @mikehearn. Fixed in #394 (merged)
  • Remove assertion that pay-to-pubkey (unhashed) is used in all coinbase txes and is more convenient. Fixed in #394 (merged)

saivann and others added some commits May 10, 2014

Contributions by @saivann to devel docs
Thanks also (in alphabetical order) to @cbeams, @mikehearn, and
@tgeller, among others.

The last pre-squash commit was: c2b8d56
Contributions by @instagibbs to devel docs
Thanks also (in alphabetical order) to @cbeams, @mikehearn, and
@tgeller, among others.

The last pre-squash commit was: c2b8d56
Contributions by @harding to devel docs
Thanks also (in alphabetical order) to @cbeams, @mikehearn, and
@tgeller, among others.

The last pre-squash commit was: c2b8d56
Contributor

luke-jr commented May 11, 2014

This seems to confuse addresses with scriptPubKeys, as well as some fungibility concerns... It also uses the new abbreviation P2PH, where "p2pkh" has been more frequently used. The section on key reuse fails to mention known security risks, only mentioning security in a hypothetical context. DoS is improperly all uppercased. Also, contrary to the section on Stratum, GBT can and often does use a persistent TCP socket - although without a persistent session, which is perhaps what was intended.

Contributor

saivann commented May 11, 2014

@luke-jr: Thanks for your feedback!

scriptPubKeys / fungibility: If you can provide more details, or better, a pull req or suggested improvements, this would be appreciated.

Key reuse & security risks: @mikehearn suggested we don't mention this risk since ECDSA public keys are secure (today). But I knew you and other developers would want it, so we opted for a non-alarmist and short version that wouldn't suggest ECDSA is insecure. But if there is any specific example you'd like mentioned, feel free to mention them here.

P2PKH / DoS: That's fine with me, I can update all occurences.

Stratum / GBT: I'll let others comment about this.

Contributor

harding commented May 11, 2014

@luke-jr thank you! Two notes in addition to @saivann's response:

DOS is all uppercased to be consistent with the style used throughout the document which does not lowercase interior prepositions in captialized phrases. For example, see all the headings with prepositions. We can special case DoS or abbreviations in general, or (more radically) change the entire text and all the images. However, how do you feel about keeping the current consistent usage?

Stratum/GBT: I'll make that update.

Contributor

luke-jr commented May 11, 2014

What repository do I clone and send a merge request to?

ECDSA public keys are not necessarily secure today, given multiple signatures. Just because we've worked around every possible exploit situation doesn't mean we should pass it off as if it has no known risks... Ignoring the risks exposes developers to them, so they need to be explicitly mentioned so developers know to avoid them.

"DOS" is "Disk Operating System"

Contributor

saivann commented May 11, 2014

@luke-jr You can clone and send pull requests to the devel-docs branch on this repository (bitcoin/bitcoin.org). Edit: preferably not too much unrelated change per pull request would work better.

Contributor

mikehearn commented May 11, 2014

I'm all for mentioning the risks of poorly implementing ECDSA! But I think a blanket statement like "address reuse is bad for security" is very strong given the state of play, if I wasn't intimately familiar with the details and read this I'd be struggling to figure out how this is a problem, given the prevalence of multiple signatures and published public keys. The wider security community doesn't make any such security recommendations so it'd be weird if the Bitcoin world was not in sync with that.

But I don't feel very strongly about this issue. If someone wants to write up an explanation of what can go wrong with ECDSA and note that avoiding pubkey reuse can sometimes mitigate those failures, I won't object.

@mikehearn mikehearn commented on an outdated diff May 11, 2014

_includes/guide_contracts.md
@@ -0,0 +1,288 @@
+## Contracts
+
+{% autocrossref %}
+
+By making the system hard to understand, the complexity of transactions
@mikehearn

mikehearn May 11, 2014

Contributor

Better phrasing:

The complexity of transactions has so far worked against you by making the system harder to understand. But the benefits become clear once we start using contracts.

@mikehearn mikehearn and 2 others commented on an outdated diff May 11, 2014

_includes/guide_contracts.md
@@ -0,0 +1,288 @@
+## Contracts
+
+{% autocrossref %}
+
+By making the system hard to understand, the complexity of transactions
+has so far worked against you. That changes with contracts. Contracts are
+transactions which use the decentralized Bitcoin system to enforce financial
+agreements.
+
+Bitcoin contracts can often be crafted to minimize dependency on outside
+agents, such as the court system, which significantly decreases the risk
+of dealing with unknown entities in financial transactions. For example,
+Alice and Bob might only know each other casually over the Internet;
+they would never open a checking account together---one of them could
+pass bad checks, leaving the other on the hook. But with Bitcoin
@mikehearn

mikehearn May 11, 2014

Contributor

It's not very important, but this example is very USA centric. Cheques are basically dead inside Europe (and elsewhere too?): the only people I know who still use them are my grandparents. I wonder if we can find a more internationalised example.

@instagibbs

instagibbs May 11, 2014

Contributor

"Alice and Bob might only know each other casually over the Internet; they would never open a checking account together---one of them could
take the money without their partner's permission."

@harding

harding May 11, 2014

Contributor

Thanks, @instagibbs! This has actually already been fixed in #394 (see the todo list in the main pull request description for an updated list of problems and who's assigned to fixing them.) Thanks again!

@mikehearn mikehearn commented on an outdated diff May 11, 2014

_includes/guide_contracts.md
+Unfortunately, the merchandise gets slightly damaged in transit. Charlie
+wants a full refund, but Bob thinks a 10% refund is sufficient. They
+turn to Alice to resolve the issue. Alice asks for photo evidence from
+Charlie along with a copy of the unhashed redeemScript Bob created and
+Charlie checked.
+
+After looking at the evidence, Alice thinks a 40% refund is sufficient,
+so she creates and signs a transaction with two outputs, one that spends 60%
+of the satoshis to Bob's public key and one that spends the remaining
+40% to Charlie's public key.
+
+In the input section of the script, Alice puts her signature, a 0x00
+placeholder byte, and a copy of the unhashed serialized redeemScript
+that Bob created. She gives a copy of the incomplete transaction to
+both Bob and Charlie. Either one of them can complete it by replacing
+the placeholder byte with his signature, creating the following input
@mikehearn

mikehearn May 11, 2014

Contributor

Technically no placeholder is necessary and you can just send around raw signatures, if all sides know what the tx is meant to look like. But it's probably unimportant to mention that.

Small Changes To Devel Doc Based On Comments By @luke-jr & @mikehearn
_includes/guide_transactions.md:

* DOS expanded to "denial of service" to improve readability and avoid
  conflation with Disk Operating System. Change based on feedback from
  @luke-jr. Thanks!

_includes/guide_mining.md:

* Dropped assertion that `getblocktemplate` can't reuse an established
  socket. Change based on feedback from @luke-jr. Thanks!

_includes/guide_contracts.md:

* Dropped opening sentences to Contracts section, which were a holdover
  from when contracts was a subsection of Transactions. New opening
  sentence is now similar to the summary sentences which open all other
  sections. Change based on feedback from @mikehearn. Thanks!

* Deleted USA-centric example from second paragraph and merged remaining
  parts of the first two paragraphs into a single opening paragraph with
  no example. Change based on feedback from @mikehearn. Thanks!

* Removed mention of placeholder byte from multisig example. Change
  based on feedback from @mikehearn. Thanks!

harding added some commits May 11, 2014

Remove Assertion That Pay-To-Pubkey (Unhashed) Still Used In Coinbase…
… Txes

_includes/guide_transactions.md:

* Assertions that pay-to-public-key (unhashed) was "used in all coinbase
  transactions" and "is more convenient" have been removed: the first
  statement because its provably untrue; the second because it's debatable
  (spending unhashed keys requires fewer bytes, so it might be more
  convenient).  Based on feedback from @TierNolan.  Thanks!
Add Comparison-Based Attacks To Reasons Not To Reuse Keys
_includes/guide_transactions.md:

* Expand the security part of the Avoiding Key Reuse subsection to also
  describe why creating more than one signature with the same private
  key might be a problem. Based on feedback from @luke-jr. Thanks!
Merge pull request #394 from harding/docsupdate1
Small Changes To Devel Doc Based On Comments
Merge pull request #395 from harding/docsupdate-sigsecurity
Devel Docs: Add Comparison-Based Attacks To Reasons Not To Reuse Keys
Contributor

wbnns commented May 13, 2014

@harding Hey! Just wanted to reach out and say BIG THANKS to you for working on this! :) You are the man! Thank you and @saivann both for putting this initiative together. This is going to be a big deal! Huge thanks also to @mikehearn and @luke-jr for helping polish this up! :)

Contributor

harding commented May 13, 2014

@gwb3 Thanks! All of us who worked on the docs are pleased with the result. There's still lots more work to be done to make this guide reasonably complete, so expect many more pull requests. :-)

@harding harding referenced this pull request May 14, 2014

Merged

Replace P2PH With P2PKH #400

saivann and others added some commits May 12, 2014

Fix Off-By-One Error In HD Wallet Section
Discovered by /u/lifeboatz on Reddit.  Thanks!
Merge pull request #401 from bitcoin/devdoc-hd-offby1
Fix Off-By-One Error In HD Wallet Section
Fix Formula For Normal HD Key Derivation; Mention Ancestor Key Risk
_includes/guide_wallets.md:

* Fix formula given for normal child key derivation to state that public
  keys must also be provided to the HMAC hash function. This required
  updating both text and images.

* Add one-paragraph warning about ancestor key compromise when the
  ancestor extended public key is compromised along with a descended
  private key.  Update img/dev/en-hd-private-parent-to-private-child.*
  to help illustrate this warning.

en/developer-reference.md:

* Remove %include% of previously-removed file which caused new versions
  of Jekyll to die.
Contributor

mikehearn commented May 19, 2014

How about we get this merged? It can always be improved live.

Contributor

harding commented May 19, 2014

@mikehearn I think @saivann was planning on merging it on May 24th after asking for a final round of reviews. As the most recent bit of incorrect information in the guide was only discovered a few days ago (see #408), I think waiting another 5 days is reasonable. (Although I admit to being quite eager to see this on Bitcoin.org myself.)

Contributor

saivann commented May 19, 2014

@mikehearn @harding Agreed, thanks!

In the absence of critical feedback, this pull request will be merged on May 24th (Edit: 15:00:00 +0000). Additional feedback and reviews are welcome and appreciated.

saivann and others added some commits May 19, 2014

Merge pull request #408 from harding/hd-ancestor-compromise
Fix Images For Normal HD Key Derivation; Mention Ancestor Key Risk
Make Clearer The Benefits Of Hardened Keys And The Absence Of A Maste…
…r PubKey

As suggested by @gmaxwell (thanks!), I tried to make clearer the benefit
of hardened keys:

* Described hardened keys as a solution in the first sentence of the
  Hardened Keys subsection.

* Reordered the text so that the problem is described before the
  solution, making the presence of a solution clearer.

* Added a prefatory sentence to the description of the two key
  derivation formulas again describing the hardened formula as a
  solution.

As suggested by @vbuterin (thanks!), I added a paragraph describing that
HD wallets don't use normal derivation on the master key so they don't
have an effective master public key. (See end of the diff.)

This is a fairly large diff because of the reordering, but no new
clauses were added besides those described above.
Merge pull request #411 from bitcoin/devel-report
Add "Report An Issue" action in developer documentation toc
Several Corrections & Clarifications Suggested On IRC
**Suggested by @cbeams:**

_includes/ref_block_chain.md:

    * Mention that coinbase is the first transaction in a block.

**Suggested by @gmaxwell:**

_includes/ref_core_rpcs-abcdefg.md:

    * Mention that you need to unlock your wallet when you run out of
      keys in the keypool.

    * Remove erroneous assertion that txindex=1 would allow
      `getreceivedbyaddress` to check balances of addresses not
      belonging to this wallet.

_includes/ref_transactions.md:

    * Clarify that OP_RETURN scripts aren't usually executed because
      they always return false.

en/developer-reference.md

    * Add a warning about using block chain or mempool data in
      executable context.

saivann and others added some commits May 23, 2014

Merge pull request #416 from bitcoin/devel-docs-disclaimer
Add a temporary BETA disclaimer in devel-docs
Contributor

harding commented May 23, 2014

In anticipation of the merge tomorrow at 15:00 UTC, I made a local test merge and jekyll compile, and then ran some tests. Except for one tiny mistake (mine) fixed now in 32c8d59, everything looks good to me.

Contributor

saivann commented May 24, 2014

Merging this!

Huge thanks to @cbeams, @harding, @instagibbs, @mikehearn, and @tgeller for your work on making this possible!

Additional issues and improvements can be reported as issues or pull request on GitHub. More general discussions can take place on the mailing list:
https://groups.google.com/forum/#!forum/bitcoin-documentation

saivann added a commit that referenced this pull request May 24, 2014

Merge pull request #393 from bitcoin/devel-docs
Add Developer Guide To Bitcoin.org

@saivann saivann merged commit 2f17ee4 into master May 24, 2014

Contributor

mikehearn commented May 24, 2014

Amazing work guys. Really, really impressed by what you have accomplished.

@saivann saivann deleted the devel-docs branch May 24, 2014

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