The Bitcoin Core team recently committed BIP0064, which adds two new commands to the protocol, getutxos and utxos. The getutxos command is used to request unspent transaction information based on the given outpoints, while the utxos command is the response.
The BIP was authored by Mike Hearn who also provided the implementation for Bitcoin Core.
Now, since this affects the Bitcoin wire protocol, btcwire needs to add support for this change, even though btcd will not implement these new commands. btcwire is the bitcoin wire protocol package that btcd uses. It has 100% test coverage, meaning it can find bugs before implementing the new API into btcd.
I wrote some code to test against Bitcoin Core. This code issues a single getutxos command for outpoint bd1f9401a9c284a04353f925276af62f23f452d297eb2cc582d037064b2a795f:1 on TestNet3. However, I made the command contain 50,000 requests for this outpoint, since I prefer test-to-fail instead of test-to-pass type development.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 | package main import ( "fmt" "net" "sync" "github.com/conformal/btcwire" "github.com/davecgh/go-spew/spew" ) const ( pver = btcwire.ProtocolVersion btcnet = btcwire.TestNet3 ) var ( conn net.Conn wg sync.WaitGroup ) func readMessage() { defer wg.Done() for { msg, _, err := btcwire.ReadMessage(conn, pver, btcnet) if err != nil { fmt.Printf("ReadMessage error: %v\n", err) return } switch m := msg.(type) { case *btcwire.MsgPing: pong := btcwire.NewMsgPong(m.Nonce) err := btcwire.WriteMessage(conn, pong, pver, btcnet) if err != nil { fmt.Printf("WriteMessage error: %v\n", err) return } case *btcwire.MsgVerAck: // Ignore case *btcwire.MsgVersion: verack := btcwire.NewMsgVerAck() err = btcwire.WriteMessage(conn, verack, pver, btcnet) if err != nil { fmt.Printf("WriteMessage error: %v\n", err) return } default: spew.Dump(m) } } } func main() { var err error host := "127.0.0.1:18333" fmt.Printf("Attempting connection to %s\n", host) conn, err = net.Dial("tcp", host) if err != nil { fmt.Printf("dial error: %v\n", err) return } defer conn.Close() wg.Add(1) go readMessage() nonce, err := btcwire.RandomUint64() if err != nil { fmt.Printf("RandomUint64 error: %v\n", err) return } // Build version message version, err := btcwire.NewMsgVersionFromConn(conn, nonce, 0) if err != nil { fmt.Printf("NewMsgVersionFromConn error: %v\n", err) return } err = version.AddUserAgent("test", "0.0.1", "") if err != nil { fmt.Printf("AddUserAgent error: %v\n", err) return } // Send version message err = btcwire.WriteMessage(conn, version, pver, btcnet) if err != nil { fmt.Printf("WriteMessage error: %v\n", err) return } // Build getutxos command get := btcwire.NewMsgGetUtxos() sha, err := btcwire.NewShaHashFromStr("bd1f9401a9c284a04353f925276af62f23f452d297eb2cc582d037064b2a795f") if err != nil { fmt.Printf("NewShaHashFromStr error: %v\n", err) return } // Add outpoint index1 50000 times. for i := 0; i < btcwire.MaxOutPointsPerMsg; i++ { op := btcwire.NewOutPoint(sha, 1) err = get.AddOutPoint(op) if err != nil { fmt.Printf("AddOutPoint error: %v\n", err) return } } // Send getutxos message err = btcwire.WriteMessage(conn, get, pver, btcnet) if err != nil { fmt.Printf("WriteMessage error: %v\n", err) return } wg.Wait() } |
When run, btcwire rejects the response due to the payload being larger than the bitcoin specification, which is 32MB.
ReadMessage error: ReadMessage: message payload is too large - header indicates 202306292 bytes, but max message payload is 33554432 bytes.
Add a few loops and extra connections in the code, and you can crash Bitcoin Core with out of memory errors:
************************ EXCEPTION: St9bad_alloc std::bad_alloc bitcoin in ProcessMessages() 2014-08-26 21:02:48 ProcessMessage(getutxos, 1800004 bytes) FAILED peer=271 2014-08-26 21:02:49 ************************ EXCEPTION: St9bad_alloc std::bad_alloc bitcoin in ProcessMessages() 2014-08-26 21:02:49 ProcessMessage(getutxos, 1800004 bytes) FAILED peer=275
So, this shows that Bitcoin Core is trying to send us a little over 202MB. The bug is that Bitcoin Core does not follow its own specification of sending messages that do not exceed the maximum message size of 32MB. Secondly, I also wonder if Bitcore Core should skip duplicate outpoints in the getutxos request.
Another issue this brings up is the amount of testing, or the lack thereof, that goes into protocol changes. It is worrisome that protocol changes get merged without accompanying extensive test coverage. This is why the Conformal team puts such a strong emphasis on complete test coverage to help catch such issues that easily go unnoticed by developers.
btcd won’t be supporting the command because it is completely unauthenticated and insecure as pointed out numerous times on the initial pull request. In response to these concerns, a section entitled “Authentication” was added to the BIP which attempts to address the concerns by simply calling them out along with some potential modifications that could happen in the future. However, we prefer to wait until the things necessary to implement this functionality in a secure fashion are already in place, which Bitcoin Core should have done as well.
I won’t argue that btcd has better unit testing than Bitcoin Core. Whoever Satoshi was, he showed no familiarity with post-1995 software development techniques and that includes a complete lack of any unit tests and therefore a testable codebase. Any modern implementation should do better.
The reason the getutxo patch does not have any unit tests was not lack of trying. The reason is that Core lacks any infrastructure for unit testing at all. You cannot currently build fake chains or inject test messages and get the responses back. Making Core a fully testable codebase is a big job that nobody has really tackled even after five years. Sergio has made a start now and that’s good to see, but there’s still a lot of work left. It’s indeed this kind of grunge work that I’m hoping to crowdfund money for.
With respect to your bug report – thanks for testing, although I think you’re being excessively aggressive about it. There are lots of ways to make Bitcoin Core run out of resources. You discovered one by locating a giant output on the testnet, but this is symptomatic of a more general problem – Core has no kind of resource monitoring or scheduling beyond lots and lots of ad hoc checks. People discover issues like this every few months and that’s without any kind of comphrensive audit. The best way to fix this is with infrastructure, not memes.
For the record, Hearn : you’re a miserable piece of scum. Your “involvement” in Bitcoin consists in trying to break the thing. This isn’t some sort of secret, to anyone. Go back to the shithole you came from, douchebag, your utility to your handlers is epsilon already.
For the average innocent trying to get involved in Bitcoin this late : do your research before using the thoroughly compromised power ranger codebase that Hearn and the rest of the USG stooges are maintaining. They’re not here to help you, they are here to defraud you. They don’t work towards your freedom but towards limiting it, and they don’t work for you, they work for your enemies. There’s very good reasons nobody uses their crap, don’t let them take advantage of your naivite & ignorance to create another victim.
it’s unbelievable that developers are throwing everything and the kitchen sink into the protocol. I’d never heard of this change, which shows how fast people are pushing for poorly thought decisions that impact the entire future of Bitcoin. It’s like people have got bored with the crappy bitcoind codebase and are looking to add all their favourite new features to the protocol. Very very bad decision making be shown on that github thread.
Innovation should happen on the IMPLEMENTATION level *not* the protocol which should stay pure, focused and simple. This thread conversation shows a totally disregard in attitude for integrity and security of the Bitcoin system. Have developers lost their mind?
Amir Taaki has done, and continues to do, huge disservice to anyone serious involved in Bitcoin.
Now how come it’s just you two pieces of dogvomit trying to crowd up the discussion ? Oh, because that’s the job. ESAD.
>It [btcd] has 100% test coverage, meaning it can find bugs before implementing the new API into btcd.
I had a look at the repo here:
https://github.com/conformal/btcd
But could find no test suite. Where are the tests located?
Rich:
David Hill is referring to btcwire (https://github.com/conformal/btcwire/). The post says “btcwire is the bitcoin wire protocol package ….. It [btcwire]….”
If you go to the linked btcwire repo and click on the test coverage badge, you can review the test coverage in-depth. This is about the wire protocol, so btcwire is the package that implements that portion. Other fundamental packages are similar in their coverage such as https://github.com/conformal/btcnet and https://github.com/conformal/btcec.
Excellent feedback on XT.
See this : http://www.reddit.com/r/bitcoinxt/comments/3i67ls/code_review_on_this_commit_rule_changes_no_bip/
Commits like this need review. Lots of review and attention. I love the devs, but get eyes on this one.