MIME-Version: 1.0
References: <CAK51vgDO2Tg38XbW0pqAnO3ETJ_qf8owRsUYsTXmrf7H2yGZtw@mail.gmail.com>
In-Reply-To: <CAK51vgDO2Tg38XbW0pqAnO3ETJ_qf8owRsUYsTXmrf7H2yGZtw@mail.gmail.com>
From: Wilmer Paulino <wilmer.paulino@gmail.com>
Date: Thu, 7 Mar 2019 16:09:01 -0800
Message-ID: <CAA5BidvSesGyofaApZY43tEziHsUcOnu6108p-UYj0Lu1+PFAA@mail.gmail.com>
To: Marco Falke <falke.marco@gmail.com>, 
	Bitcoin Protocol Discussion <bitcoin-dev@lists.linuxfoundation.org>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [bitcoin-dev] Removal of reject network messages from Bitcoin
 Core (BIP61)
Precedence: list

Hi Marco,

> At this time, I am not aware of any software that requires this feature, and I
> would like to remove if from Bitcoin Core to make the codebase slimmer, easier
> to understand and maintain.

Neutrino[1], a light client implementation that uses BIPs 157 and 158,
currently relies on receiving reject messages from its peers when broadcasting
a transaction to the network. I've personally gone through the relevant parts
of the Bitcoin Core codebase involving reject messages and respectfully
disagree that removing it would help much in terms of comprehension and
maintainability. IMO, the benefits outweigh this small cost.

> Nodes on the network can not generally be trusted to send valid ("reject")
> messages, so this should only ever be used when connected to a trusted node.

Nodes in the network generally rely on the assumption that they are connected
to at least one honest peer, so we can actually converge on the set of honest
peers and ban/disconnect any who send an invalid reject message for a valid
transaction.

> Let us know if your application relies on this feature and you can not use any
> of the recommended alternatives:

Unfortunately, none of the recommended alternatives work for our use case. The
main thing we want to identify when broadcasting a transaction is whether is
it invalid or not. As long as it is valid, reject messages aren't required as
the light client can just rebroadcast the transaction upon every new block to
ensure it is eventually included in the chain. It can then stop rebroadcasting
it once it detects it has confirmed on-chain through its filters. However, if
it is invalid, some of the validity checks required cannot be performed by
light clients as they do not have a mempool and/or UTXO set.

Reject messages also useful when developing new light clients, as we can get
some feedback from the network on why a transaction was rejected, which helps
identify potential bugs in their transaction crafting logic. I understand that
this can be done by setting up test nodes with the flag enabled, but this
justifies that the feature should at least exist and not be completely
removed.

> * Testing the validity of a transaction can be achieved by specific RPCs:
>  - `sendrawtransaction`
>  - `testmempoolaccept`

These RPCs are not helpful for light clients. Even for full nodes, in the case
of `testmempoolaccept`, mempool conditions can quickly change and cause a
transaction to be rejected after the fact. One alternative would be for a
third-party to set up an endpoint where users can submit their transactions
to, but now you're placing your trust solely on them, rather than the network,
which doesn't seem like a reasonable or comparable compromise.

With that said, I believe the feature should remain enabled by default in
order to aid the light clients of the network. If we disable them by default,
no one will bother to enable them manually, and light clients won't be able to
realize they are broadcasting invalid transactions.

[1] https://github.com/lightninglabs/neutrino

- Wilmer