Date: Wed, 27 Jun 2018 13:55:59 -0400
To: William Casarin <jb55@jb55.com>
From: Achow101 <achow101-lists@achow101.com>
Reply-To: Achow101 <achow101-lists@achow101.com>
Message-ID: <0BT4A0BFbcfUM9xlYjS-7Cy1zpaI1J9qsIpWH_xgv2ZLhcmxb4Es5KlpMJCvHVEu8BDbBweZ92RHnES5HxDMulRhJkYSZAPi-CgXQ3uwkfY=@achow101.com>
In-Reply-To: <87k1qk7oca.fsf@jb55.com>
References: <CAPg+sBhGMxXatsyCAqeboQKH8ASSFAfiXzxyXR9UrNFnah5PPw@mail.gmail.com>
	<20180621195654.GC99379@coinkite.com>
	<CAPg+sBgdQqZ8sRSn=dd9EkavYJA6GBiCu6-v5k9ca-9WLPp72Q@mail.gmail.com>
	<ljk5Z_a3KK6DHfmPJxI8o9W2CkwszkUG34h0i1MTGU4ss8r3BTQ3GnTtDTfWF6J7ZqcSAmejzrr11muWqYN-_wnWw_0NFn5_lggNnjI0_Rc=@achow101.com>
	<f8f5b1e3-692a-fc1e-2ad3-c4ad4464957f@satoshilabs.com>
	<TGyS7Azu3inMQFv9QFn8USr9v2m5QbhDRmiOI-4FWwscUeuIB9rA7mCmZA4-kwCJOMAx92fO7XICHtE7ES_QmIYLDy6RHof1WLALskGUYAc=@achow101.com>
	<c32dc90d-9919-354b-932c-f93fe329760b@satoshilabs.com>
	<J0KV-aP96fSVHPkPw85N2qdKV_F5vqXt5fIFwKDp9wBjRKJ6bZpUEtzbgHyxlWW9PCXMOEVZnyUnJ-kW281ZbblbCp2sbZI_UyTP46q-PiY=@achow101.com>
	<87k1qk7oca.fsf@jb55.com>
Feedback-ID: VjS95yl5HLFwBfNLRqi61OdL1ERZPmvMbZRH2ZcBR7SKVUVYPgv7VJsV9uoyC4vIfjYnW8hPXGuLTycZbh49Zw==:Ext:ProtonMail
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Cc: Bitcoin Protocol Discussion <bitcoin-dev@lists.linuxfoundation.org>
Subject: Re: [bitcoin-dev] BIP 174 thoughts
Precedence: list

Hi,=E2=80=8B

On June 26, 2018 11:09 PM, William Casarin <jb55@jb55.com> wrote:

> =E2=80=8B=E2=80=8B
>=20
> Hey Andrew,
>=20
> If I'm reading the spec right: the way it is designed right now, you
>=20
> could create hundreds of thousands of zero bytes in the input or output
>=20
> key-value arrays. As far as I can tell this would be considered valid,
>=20
> as it is simply a large array of empty dictionaries. Is this right? I'm
>=20
> worried about buffer overflows in cases where someone sends a large blob
>=20
> of zeros to an unsuspecting implementation.

No, that is incorrect. That whole paragraph is actually outdated, it was in=
tended
for the possibility of adding output maps, which we have already done. I ha=
ve=20
removed it from the BIP.

However, it is possible for a PSBT to contain very large unknown key-value =
pairs=20
which could potentially cause a problem. But I do not think that large PSBT=
s should=20
really be a problem as they are really something that the user has to enter=
 rather=20
than something received remotely without user interaction.


On June 27, 2018 6:39 AM, Andrea via bitcoin-dev <bitcoin-dev@lists.linuxfo=
undation.org> wrote:

> =E2=80=8B=E2=80=8B
>=20
> Hi William, Andrew, list,
>=20
> As noted by William there are some types missing in the global-types defi=
nition, because the number of each map for I/O must be known to the parser =
in order to use the correct definitions for the types. At the moment a pars=
er reading a key-value record does not know whether it should read it as pe=
r-input type or per-output, a way to address this is to declare in advance =
the number of maps and ensure they are ordered (inputs first). If you haven=
't already worked out some types for that i propose using:
>=20

Parsers actually do know because that information is present in the unsigne=
d transaction=20
at the beginning of each PSBT. Since each input and output must be accounte=
d for,
a parser can just parse the unsigned transaction and from there it can know=
 how
many inputs and outputs to expect. If it sees more or less, it should throw=
 an error
as the transaction is invalid.

Of course this implies that implementations will need to parse the unsigned=
 transaction,
but not all actually need to. Combiners do not need to, they just need to m=
erge the
maps together and follow the key uniqueness rule. They don't really need to=
 know
or care about the number of inputs and outputs, just that the PSBTs being m=
erged
share the same unsigned transaction and have the same number of maps.

Other roles need to understand the unsigned transaction anyways, so they st=
ill need
to parse it thus this isn't really a problem for those roles.

>    =20
>     On another note I think we can set a hard limit on the size of the PS=
BT, currently is 'legal' to produce a very large PSBT with an excessive num=
ber of Inputs and Outputs. By excessive I mean that even in the best case s=
cenario (using the smallest possible scriptPubKey as in P2WPKH) it is possi=
ble to create a PSBT that would certainly create an invalid transaction (be=
cause of its size) when finalized. I haven't found anything related to this=
 in the previous discussions, please ignore this if it was already proposed=
/discussed.
>    =20

I don't think such a limitation is practical or useful. A transaction can c=
urrently have, at most,
~24000 inputs and ~111000 outputs (+/- a few hundred) which is well beyond =
any useful limit.
Additionally, such limits may not be as extensible for future work. It is h=
ard to determine what
is a reasonable limit on transaction size, and I don't think it is useful t=
o have a limit. At worst
we would simply create an invalid transaction if it were too large.


Andrew