public inbox for bitcoindev@googlegroups.com
 help / color / mirror / Atom feed
From: Achow101 <achow101-lists@achow101•com>
To: William Casarin <jb55@jb55•com>
Cc: Bitcoin Protocol Discussion <bitcoin-dev@lists•linuxfoundation.org>
Subject: Re: [bitcoin-dev] BIP 174 thoughts
Date: Wed, 27 Jun 2018 13:55:59 -0400	[thread overview]
Message-ID: <0BT4A0BFbcfUM9xlYjS-7Cy1zpaI1J9qsIpWH_xgv2ZLhcmxb4Es5KlpMJCvHVEu8BDbBweZ92RHnES5HxDMulRhJkYSZAPi-CgXQ3uwkfY=@achow101.com> (raw)
In-Reply-To: <87k1qk7oca.fsf@jb55.com>

Hi,​

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

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

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

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



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

> ​​
> 
> Hi William, Andrew, list,
> 
> As noted by William there are some types missing in the global-types definition, 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 parser reading a key-value record does not know whether it should read it as per-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:
> 

Parsers actually do know because that information is present in the unsigned transaction 
at the beginning of each PSBT. Since each input and output must be accounted 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 merge 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 merged
share the same unsigned transaction and have the same number of maps.

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

>     
>     On another note I think we can set a hard limit on the size of the PSBT, currently is 'legal' to produce a very large PSBT with an excessive number of Inputs and Outputs. By excessive I mean that even in the best case scenario (using the smallest possible scriptPubKey as in P2WPKH) it is possible to create a PSBT that would certainly create an invalid transaction (because 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.
>     

I don't think such a limitation is practical or useful. A transaction can currently 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 hard to determine what
is a reasonable limit on transaction size, and I don't think it is useful to have a limit. At worst
we would simply create an invalid transaction if it were too large.


Andrew




  parent reply	other threads:[~2018-06-27 17:56 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-15 23:34 Pieter Wuille
2018-06-16 15:00 ` Peter D. Gray
2018-06-19  9:38 ` Jonas Schnelli
2018-06-19 14:20 ` matejcik
2018-06-19 15:20   ` Jonas Schnelli
2018-06-21 20:28     ` Peter D. Gray
2018-06-19 17:16   ` Pieter Wuille
2018-06-21 11:29     ` matejcik
2018-06-21 17:39       ` Pieter Wuille
2018-06-21 11:44     ` Tomas Susanka
2018-06-19 14:22 ` matejcik
2018-06-21  0:39 ` Achow101
2018-06-21 14:32   ` Tomas Susanka
2018-06-21 15:40     ` Greg Sanders
2018-06-21 19:56     ` Peter D. Gray
2018-06-21 21:39       ` Gregory Maxwell
2018-06-22 19:10       ` Pieter Wuille
2018-06-22 22:28         ` Achow101
2018-06-23 17:00           ` William Casarin
2018-06-23 20:33             ` Andrew Chow
2018-06-24  8:19               ` Andrea
2018-06-24  8:28                 ` Andrew Chow
2018-06-24  9:00                   ` Andrea
2018-06-23 18:27           ` Peter D. Gray
2018-06-25 19:47           ` Tomas Susanka
2018-06-25 20:10             ` Jonas Schnelli
2018-06-25 20:30             ` Achow101
2018-06-26 15:33               ` matejcik
2018-06-26 16:58                 ` William Casarin
2018-06-26 17:11                   ` Marek Palatinus
2018-06-27 14:11                   ` matejcik
2018-06-26 20:30                 ` Pieter Wuille
2018-06-27 14:04                   ` matejcik
2018-06-27 15:06                     ` Pieter Wuille
2018-06-29  9:53                       ` matejcik
2018-06-29 19:12                         ` Achow101
2018-06-29 20:31                           ` Peter D. Gray
2018-07-04 13:19                           ` matejcik
2018-07-04 18:35                             ` Achow101
2018-07-05 17:23                               ` Jason Les
2018-07-04 19:09                             ` Pieter Wuille
2018-07-05 11:52                               ` matejcik
2018-07-05 22:06                                 ` Pieter Wuille
2018-07-10 12:10                                   ` matejcik
2018-07-11 18:27                                     ` Pieter Wuille
2018-07-11 20:05                                       ` Gregory Maxwell
2018-07-11 20:54                                         ` [bitcoin-dev] BIP 174 thoughts on graphics vv01f
2018-06-26 21:56                 ` [bitcoin-dev] BIP 174 thoughts Achow101
2018-06-27  6:09                   ` William Casarin
2018-06-27 13:39                     ` Andrea
2018-06-27 17:55                     ` Achow101 [this message]
2018-06-28 20:42                       ` Rodolfo Novak
2018-07-05 19:20                       ` William Casarin
2018-07-06 18:59                         ` Achow101
2018-06-20  0:39 Jason Les

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='0BT4A0BFbcfUM9xlYjS-7Cy1zpaI1J9qsIpWH_xgv2ZLhcmxb4Es5KlpMJCvHVEu8BDbBweZ92RHnES5HxDMulRhJkYSZAPi-CgXQ3uwkfY=@achow101.com' \
    --to=achow101-lists@achow101$(echo .)com \
    --cc=bitcoin-dev@lists$(echo .)linuxfoundation.org \
    --cc=jb55@jb55$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox