From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 9C055720 for ; Wed, 29 Jul 2015 21:46:52 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-wi0-f171.google.com (mail-wi0-f171.google.com [209.85.212.171]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 05649228 for ; Wed, 29 Jul 2015 21:46:50 +0000 (UTC) Received: by wicmv11 with SMTP id mv11so235348233wic.0 for ; Wed, 29 Jul 2015 14:46:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=jcjuA9Dr5Ac+p/jUNYUc/exHNqySGOxOw6T6OejpWtQ=; b=jZrxYGAzfDav+X58sYZ/qyg2U0Zlpa3gYyv/4vfkz0bb3SwV8KCzgJL4UGFB1TmYta pyXMENH1jJRkrIBRFRG0oWvXbTZ07EikcCL9H+d4okP9QsfN2fpJEDvmXjr5DswR6kzA g74TXNCnNThiOyTVF98JfE2sMc2udj5GMMGw7EwsA1Tu9VAxnUapj/ZM576syT30+fem 81Ic/vRJVe4OrTR+dLdO5O5Hwn6yQqbLknglCgXGakkow0DAbRnQuL+awIelsk/Os8zZ mqm9tO6shgoPaOT5yR62n4cMW+0Orrvp7OVb4YS7Cxnuqg70XZE/gq3ZUCfl8zlv7Vve 9VrQ== X-Gm-Message-State: ALoCoQncod0jzKoP1rkKt2KW9NB1/ub5EOqnMN8t2ozzaZarQuBscARq1+vU2Uu8ok1hCHbt09Cu MIME-Version: 1.0 X-Received: by 10.194.187.170 with SMTP id ft10mr80418370wjc.26.1438206409615; Wed, 29 Jul 2015 14:46:49 -0700 (PDT) Received: by 10.194.95.168 with HTTP; Wed, 29 Jul 2015 14:46:49 -0700 (PDT) In-Reply-To: <55B939CF.1080903@voskuil.org> References: <55B723EA.7010700@voskuil.org> <55B939CF.1080903@voskuil.org> Date: Wed, 29 Jul 2015 23:46:49 +0200 Message-ID: From: =?UTF-8?B?Sm9yZ2UgVGltw7Nu?= To: Eric Voskuil Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Bitcoin Dev Subject: Re: [bitcoin-dev] Libconsensus separated repository (was Bitcoin Core and hard forks) X-BeenThere: bitcoin-dev@lists.linuxfoundation.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Bitcoin Development Discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Jul 2015 21:46:52 -0000 On Wed, Jul 29, 2015 at 10:38 PM, Eric Voskuil wrote: > On 07/28/2015 02:58 AM, Jorge Tim=C3=B3n wrote: > Oh, I misunderstood your ask then. I don't have a preference on > prioritizing VerifyTx vs VerifyHeader. Ok, let's assume we want to expose verifyHeader first (which I think will be easier). >> would this be an acceptable way to expose >> VerifyHeader? > > I'm not sure how you mean to expose it, could you clarify? In https://github.com/bitcoin/bitcoin/pull/5995 I had one (probably stupid) proposal. But it had so many preparations commits that I had to close it. In the last commit https://github.com/jtimon/bitcoin/commit/00b9b227afc8669a877984561329dde75d= 3d8942 you can see that I'm adding a new function in script/bitcoinconsensus.cpp with the following declaration: int bitcoinconsensus_verify_header(const unsigned char* blockHeader, unsigned int blockHeaderLen, const Consensus::Params& params, int64_t nTime, CBlockIndexBase* pindexPrev, PrevIndexGetter indexGetter, bitcoinconsensus_error* err) The ugly parts that you may not like are the CBlockIndexBase struct (or maybe it's not so unreasonable) and the function pointer PrevIndexGetter. To see their "ugliness" you can look at: https://github.com/jtimon/bitcoin/commit/4528ec69617f1b6d6c8f0d73dc4091cded= 7c216c The PrevIndexGetter function pointer that Bitcoin Core would use internally would be: const CBlockIndexBase* GetPrevIndex(const CBlockIndexBase* pindex) { return ((CBlockIndex*)pindex)->pprev; } with an ugly casting. But, well, I guess that's only ugly for Bitcoin Core, not necessarily for other libconsensus users, which can define their own function pointer, provided that it's of the form: typedef const CBlockIndexBase* (*PrevIndexGetter)(const CBlockIndexBase*); The struct that I think needs more refinement (and I just used what I considered easier to implement at the time) is the CBlockIndexBase struct itself: +struct CBlockIndexBase +{ + //! pointer to the hash of the block, if any. Memory is owned by this CBlockIndexBase + const uint256* phashBlock; + //! block header + int32_t nVersion; + uint256 hashMerkleRoot; + uint32_t nTime; + uint32_t nBits; + uint32_t nNonce; + //! height of the entry in the chain. The genesis block has height 0 + int nHeight; +}; I don't like phashBlock being a pointer instead of just a ref or even an ob= ject Should that struct have a CBlockIndexBase* pprev; field (moving it down from CBlockIndex)? That's the kind of question where your feedback seems very important from other-implementations developers (because you won't necessarily take into account the difficulty of the refactors required in Bitcoin Core to expose the right interface, and "libconsensus shouldn't care" either, all we want is the best interface). >> Which of he step-checks functions is worth exposing too (Bitcoin >> Core is currently using some to prevent DoS attacks, for example)? > > I don't see any reason to expose checkpoints in this library. They are > trivial to implement and are not part of consensus. Agreed, and I would say all of the checkpoint check separation has been done already. What I mean by step functions is...look at verfyHeader internals, for examp= le: https://github.com/jtimon/bitcoin/commit/11ede96f59f611ede596a1335e896b1fef= 4fb5b2 It internally calls Consensus::CheckBlockHeader (quite cheap with no context required) and Consensus::ContextualCheckBlockHeader (not so cheap). Bitcoin Core never calls (yet) the full verifyHeader at once. It does the cheap tests first and the expensive later. For example, call CheckBlockHeader, then CheckBlock (also cheap), then ContextualCheckBlockHeader and then ContextualCheckBlock. The question is, will other implementations want access to these not-full-but-cheap tests? In other words, apart from exposing VerifyHeader that fully validates all consensus rules for a header, do we also want to expose CheckBlockHeader and ContextualCheckBlockHeader to give more flexibility to libconsensus' users? I think, yes, other implementations will want this for the same DoS reasons that Bitcoin Core currently wants them. But it would be nice to know what a second person thinks about this. > Nothing can eliminate all consensus risk, not even a common full node > implementation. In fact, one thing does: never changing the code again (but the cure would be worse than the illness). Agreed, any software changes in the consensus code can cause consensus forks (and that's why you don't want to touch libconsensus that much once it's separated). >>> Useful specifications often have two reference implementations. It's th= e >>> idea that there can be only one legitimate implementation that's >>> problematic. >> >> Well, this is where I fear we will never agree. I think "Bitcoin is >> different" in this reward and you disagree. >> Maybe Pieter's explanation is more convincing to you: >> https://youtu.be/PxW5D9xCIsc?t=3D769 >> Otherwise, I think I'll stop trying convincing you. > > Maybe I wasn't sufficiently explicit. It is problematic. That is the > core issue we are dealing with. That doesn't mean I disagree with the > objectives of an independent community consensus library. > > The premise of the "one true library" idea is that there is *no way* to > sufficiently test for consensus bugs in any software release. That of > course means that each release of the satoshi client poses a significant > risk to the network. This risk is presently greater than that posed by > other implementations simply because of adoption. That is the basis of > the red herring argument: Well, the "one true library" will be much better than the current "one true full node". The "one true library" would be the specification of the consensus rules, but that doesn't mean you can't fork and modify it however you want. > The bottom line is that nobody has control over this process. There are, > and will always be, a multitude of consensus implementations that intend > to target the same coin. Presently there are multiple versions of the > satoshi client, and this has produced forks, and will continue to do so. I get this point, even if the current satoshi client contains the consensus rules specification (and many other things, obviously), that doesn't mean is somehow protected from forking with itself if the consensus code is changed in the wrong way accidentally. But the more separated libconsensus and Bitcoin Core (satoshi client) are, the less likely that changes in Bitcoin Core that weren't supposed to change consensus rules actually do it by accident (like last time with the migration out of bdb). > Isolating the satoshi consensus checks to an independent library serves > not to eliminate that risk, but can reduce it somewhat. Importantly it > will allow various implementations to overcome a perception problem, > which will improve implementation diversity and developer participation. I think alternative implementations using a full libconsensus can increase their adoption a lot, since they become just as vulnerable to consensus forks as Bitcoin Core (instead of more vulnerable like now). >>>> I believe that's the only point where we fundamentally disagree, but >>>> it shouldn't be a barrier in our common goal of taking "power" away >>>> from Bitcoin Core development. If we're successful Bitcoin Core won't >>>> have any privileged position with regards to, say, libbitcoin when it >>>> comes to deciding consensus rules changes. >>> >>> I don't think we disagree on anything fundamental. My issues with the >>> library were (1) the lack of isolation, (2) the fact that the satoshi >>> client wouldn't actually use the library, and (3) backtracking to use >>> OpenSSL, which we had recently removed from libbitcoin. >> >> 1) Working on it > > For the sake of clarity, this is now a non-issue for us. You mean libbitcoin's code is better organized than Bitcoin Core's? I don't doubt it. Maybe we can create a full-libbitcoin-libconsensus first and work on the API there. >> 2) The Satoshi client has been using all along and it will use it >> forever (maybe not through the API, but I don't get what the problem >> with that is). > > Again, I consider this a requirement for us to link directly to it as a > library. If the sources are isolated into an independent repo, but the > satoshi client is embedding its own copies, one must continue to diff > the client sources against the library sources. We are doing this > already, so the benefit to having the independent repo is in no longer > having to do this. Oh, I see, you don't like that libsecp256k1 is currently a subtree of Bitcoin Core either for the same reasons, right? To not need to know when the changes in libconsensus are applied in Bitcoin Core. Mhmm, once libconsensus is complete, why would you care about it? You just care about the libconsensus version (which doesn't have to coincide with Bitcoin Core versions anymore). > There are also differences in the build system that can affect outcome. > Comparing those differences across repos can be more challenging. For > this reason I consider it important to your objective that the satoshi > client actually use the library - as I assume it will at some point. For the sake of clarity, please say "use the library's API". It's going to use the library one way or another. > If the satoshi client folks are to maintain a consensus library for the > community it's also important to show a commitment to its independence. > Dogfooding is of course a software engineering best practice. But there > is also the cynical perspective - the independent library in some ways > works against an advantage of the satoshi client. > > I personally don't think the committers are parochial enough to let this > become an issue. We are all after something bigger. But if there was > push-back against using the library it would be a red flag. So until > that point passes I would just maintain our independent library, cloning > the sources from the satoshi client. To be clear, I don't oppose to "dogfooding", it's just clear to me that it will take even longer. So what I don't understand is "once libbitcoin is complete and ready for us to use, we will keep using our reimplementation of consensus until Bitcoin Core uses the API as well. If Bitcoin core doesn't use the API, we prefer not to use the library at all and keep having the same consensus risk. We will do what we think it's worse for us until Bitcoin Core uses the library through the API". >> 3) There will be an announce about this soon. > > Yes, I've seen this as a temporary setback. And we will hopefully migrate the current libconsensus from openSSL to libsecp256k1 soon. So we will be able to enjoy libsecp256k1's performance improvements without risking consensus. One problem less. >>> Always willing to work with you on it, although we're all busy, and thi= s >>> isn't my top priority presently. >> >> Is it because "fear of consensus bugs is what keeps people on the >> satoshi client" and you want to keep things this way? > > No, I see it as less significant to the adoption of libbitcoin-server > than other issues we are working on, especially given the existence of > libbitcoin-consensus. I also trust you will make progress regardless. This was just a joke because you said something similar earlier. Don't take it seriously.