Do not accept immature name_firstupdate into mempool
Opened this issue · 7 comments
Currently, name_firstupdate transactions are valid for mempool acceptance, as long as the name_new has at least 1 confirm.
It would be preferable should they only be accepted if the underlying name_new is mature, since if it's possible to send immature name_firstupdate transactions, people might, and if they do, miners could front-run their names. This would also simplify transaction queue logic immensely.
That's an interesting suggestion, especially due to the possible mempool simplifications. I don't think we should prevent people from broadcasting their name_firstupdate early if they want to simply because it is risky; if they do that, then they should know what they are doing, and perhaps they are fine with taking some risk. It can be handy to broadcast the name_firstupdate after a few (but not 12) name_new confirmations if you trust that no long reorg is going to happen.
Do you have an estimate of how much the code would be simplified (e.g. in an example PR)? I think that would be useful to judge whether or not we want to do this at all.
Concept ACK for 2 reasons:
- AFAIK the reason why Vince allowed the mempool to hold
name_firstupdatetransactions that are currently invalid to mine boils down to "some users don't care about security; we should give them the freedom to shoot themselves in the foot." The cryptoengineering field has advanced a lot since 2011; it is currently well-understood in cryptoengineering that exposing footguns to the user is an antipattern and should always be avoided. (See: why the Go standard library is considered to be competently designed and OpenSSL is considered to be incompetently designed.) - Allowing the mempool to hold things that aren't eligible to be mined violates a lot of the assumptions that Bitcoin code makes, and requires extra complexity. This complexity happened to manifest in a bug a few years ago that caused most hashrate to brick itself for several days. During that period, only 2 major mining pools were able to mine properly, meaning that Namecoin users during that period had basically no protection against double-spends. It would be just highly unfortunate if that complexity existed for a legitimate reason, but the reality is that the illegitimate footgun use case (see point 1) caused a negative externality (in the form of complexity and a resulting bug) that broke security for users who didn't opt into that footgun use case.
Disclosure: This came to my attention during a discussion with @yanmaani about the tranasaction queue work that he's doing for NLnet. We initially determined that the queue API could not be simplified without eliminating immature name_firstupdate from mempool admission. I later realized that you can work around this by setting an nSequence timelock on the name_new input in the name_firstupdate transaction, so I don't think this issue is still relevant for transaction queue purposes (@yanmaani is welcome to correct me if I'm mistaken). However, I still think we should fix it, for the reasons above.
- is certainly a valid point, and also what the "code simplifications" boil down to - so I'd be interested to see an example PR of how much it really simplifies the code.
I mostly agree with 1), although it has to be mentioned that this feature is only exposed through the RPC / CLI interface. You can shoot yourself in the foot with that pretty badly already (e.g. dump your private key and send it to some random guy who asks you to execute commands you don't understand). Also sending a name_firstupdate after e.g. two or six confirmations for convenience is definitely not something I would call "insecure", it is just marginally less secure - and I can imagine there are users out there who do exactly this, whom we would be breaking with a change like this.
Theoretically, the issue is that miners might pressure users into sending name_update immediately if this is possible, i.e. by refusing to mine name_new transactions without associated name_update. Although it seems like a fairly weak attack, since if they need 1 confirm anyway, they'd need to reorg it, which is much more expensive and doesn't allow for this attack. However, it seems like there's some vague harm to the ecosystem done by sending name_firstupdate early.
I think a reasonable compromise might be to ban it on the policy layer, but not on the consensus layer. This should give us the best of both worlds.
Though I also see Daniel's point. There's probably some people who rely on this, the harm done from it seems fairly minimal, and the original incentive is no longer there.
How about this? For a real grade A kludge, have name_firstupdate check if the output would be immature. If so, silently pass it over to queuerawtransaction instead. Users won't notice a thing, but internal code gets simplified.
Yes, I think the transaction queue is a good replacement for these use cases. So I suggest to get it finished first, and then I can prepare a PR that disallowed immature name_firstupdate in the mempool to see how much of a code simplification we can get. And then as the last step we can think about changing name_firstupdate in the way you suggest.
(BTW, all of this is only the policy layer. In the consensus layer, you also now can't get name_firstupdate mined before the name_new has enough confirmations.)
It is ready.
Yes, I mean, including my time to review it and merge it all. :)