Return-Path: Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 70736CB0 for ; Tue, 26 Jun 2018 21:56:31 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from mail2.protonmail.ch (mail2.protonmail.ch [185.70.40.22]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 967E012E for ; Tue, 26 Jun 2018 21:56:30 +0000 (UTC) Date: Tue, 26 Jun 2018 17:56:26 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=achow101.com; s=protonmail; t=1530050188; bh=0J6wL2rXZuZji5AC3Pt49k1Si9n8/uWxMna/evEc22s=; h=Date:To:From:Reply-To:Subject:In-Reply-To:References:Feedback-ID: From; b=Rp9ClehCaPqlwFPsm0M89pagLo7kTlscB1DYtC+fU9s+dDgoLQ8zFb2FETBbonjg9 aiXb/osqVOG5BN1y+hDtSC8q3XvWED97cH5hWsndM4fic/4E5PxBSF4DKgf85UddgJ Cx1fwwL9l/voUh3bZZ3ifwMTvsBnUbb35z1Xl+ZQ= To: matejcik , Bitcoin Protocol Discussion From: Achow101 Reply-To: Achow101 Message-ID: In-Reply-To: References: <21a616f5-7a17-35b9-85ea-f779f20a6a2d@satoshilabs.com> <20180621195654.GC99379@coinkite.com> Feedback-ID: VjS95yl5HLFwBfNLRqi61OdL1ERZPmvMbZRH2ZcBR7SKVUVYPgv7VJsV9uoyC4vIfjYnW8hPXGuLTycZbh49Zw==:Ext:ProtonMail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, 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 Subject: Re: [bitcoin-dev] BIP 174 thoughts X-BeenThere: bitcoin-dev@lists.linuxfoundation.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Bitcoin Protocol Discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Jun 2018 21:56:31 -0000 Hi, On June 26, 2018 8:33 AM, matejcik via bitcoin-dev wrote: > =E2=80=8B=E2=80=8B >=20 > hello, >=20 > in general, I agree with my colleague Tomas, the proposed changes are >=20 > good and achieve the most important things that we wanted. We'll review >=20 > the proposal in more detail later. >=20 > For now a couple minor things I have run into: >=20 > - valid test vector 2 ("one P2PKH input and one P2SH-P2WPKH input") > =20 > seems broken, at least its hex version; a delimiter seems to be missi= ng, > =20 > misplaced or corrupted Fixed > =20 > - at least the first signing vector is not updated, but you probably > =20 > know that I updated all of the tests yesterday so they should be correct now. I will = be adding more tests this week. > =20 > - BIP32 derivation fields don't specify size of the "master public key"= , > =20 > which would make it un-parsable :) Oops, that's actually supposed to be master key fingerprint, not master pub= lic key. I have updated the BIP to reflect this. > =20 > - "Transaction format is specified as follows" and its table need to be > =20 > updated. Done. > =20 > I'm still going to argue against the key-value model though. > =20 > It's true that this is not significant in terms of space. But I'm mor= e > =20 > concerned about human readability, i.e., confusing future implementer= s. > =20 > At this point, the key-value model is there "for historical reasons", > =20 > except these aren't valid even before finalizing the format. The > =20 > original rationale for using key-values seems to be gone (no key-base= d > =20 > lookups are necessary). As for combining and deduplication, whether k= ey > =20 > data is present or not is now purely a stand-in for a "repeatable" fl= ag. > =20 > We could just as easily say, e.g., that the high bit of "type" specif= ies > =20 > whether this record can be repeated. > =20 > (Moreover, as I wrote previously, the Combiner seems like a weirdly > =20 > placed role. I still don't see its significance and why is it importa= nt > =20 > to correctly combine PSBTs by agents that don't understand them. If y= ou > =20 > have a usecase in mind, please explain. There is a diagram in the BIP that explains this. The combiner's job is to = combine two PSBTs that are for the same transaction but contain different data such as signatures.= It is easier to implement a combiner that does not need to understand the types at all, and such comb= iners are forwards compatible, so new types do not require new combiner implementations. > =20 > ISTM a Combiner could just as well combine based on whole-record > =20 > uniqueness, and leave the duplicate detection to the Finalizer. In ca= se > =20 > the incoming PSBTs have incompatible unique fields, the Combiner woul= d > =20 > have to fail anyway, so the Finalizer might as well do it. Perhaps it > =20 > would be good to leave out the Combiner role entirely?) A transaction that contains duplicate keys would be completely invalid. Fur= thermore, in the set of typed records model, having more than one redeemScript and witnessScript should b= e invalid, so a combiner would still need to understand what types are in order to avoid this situat= ion. Otherwise it would produce an invalid PSBT. I also dislike the idea of having type specific things like "only one redee= mScript" where a more generic thing would work. > =20 > There's two remaining types where key data is used: BIP32 derivations > =20 > and partial signatures. In case of BIP32 derivation, the key data is > =20 > redundant ( pubkey =3D derive(value) ), so I'd argue we should leave = that > =20 > out and save space.=20 I think it is still necessary to include the pubkey as not all signers who = can sign for a given pubkey may know the derivation path. For example, if a privkey is imported into a wall= et, that wallet does not necessarily know the master key fingerprint for the privkey nor would it necessarily ha= ve the master key itself in order to derive the privkey. But it still has the privkey and can still sig= n for it. > =20 > Re breaking change, we are proposing breaking changes anyway, existin= g > =20 > code will need to be touched, and given that this is a hand-parsed > =20 > format, changing `parse_keyvalue` to `parse_record` seems like a smal= l > =20 > matter? The point is to not make it difficult for existing implementations to chang= e. Mostly what has been done now is just moving things around, not an entire format change itself. Changing to a set= of typed records model require more changes and is a complete restructuring of the format, not just moving thin= gs around. Additionally, I think that the current model is fairly easy to hand parse. = I don't think a record set model would make it easier to follow. Furthermore, moving to Protobuf would make it harder t= o hand parse as varints are slightly more confusing in protobuf than with Compact size uints. And with the key-value = model, you don't need to know the types to know whether something is valid. You don't need to interpret any data. Andrew