search

namecoin / namecoin-core

Namecoin full node + wallet based on the current Bitcoin Core codebase.
https://www.namecoin.org/
MIT License
455 stars 146 forks source link

RBF may be broken for name transactions #278

Open JeremyRand opened 5 years ago

JeremyRand commented 5 years ago

Upstream Electrum recently added RBF batching support, and I'm trying to adapt that feature to Electrum-NMC. What (I think) I'm doing is the following:

  1. Create and broadcast a name transaction "X" with RBF enabled.
  2. Take all currency inputs, all currency outputs, and name inputs of transaction X, and put them into a new transaction "Y" (also with RBF enabled).
  3. Add a name output to transaction Y, which is the same name as transaction X but a different value.
  4. Bump the fee of transaction Y compared to transaction X.
  5. Broadcast transaction Y.

I would expect transaction Y to be broadcast without errors, and replace transaction X in the mempool. Instead, I'm getting the transaction was rejected by network rules (code 0). The similarity in this error message to what's reported in https://github.com/namecoin/namecoin-core/issues/50 makes me suspect that Namecoin Core is doing the check for existing unconfirmed name transactions (and therefore rejecting the transaction from the mempool) before it takes RBF into account. I'm not 100% confident in this assessment, but I figured I'd file a bug since I suspect that @domob1812 can figure out what's happening on the Namecoin Core side of things more easily than I can.

My Namecoin Core node is v0.17.99.0-9d496c093, running on a hybrid of Debian Stretch and Whonix 14 on ppc64le.

domob1812 commented 5 years ago

I've not looked into that yet in detail, but yes, it is very well possible that RBF is broken due to the name-specific mempool checks. I suspect this is not high priority for Namecoin (as we don't have full blocks), but is of course good to fix nevertheless. Thanks for spotting!

JeremyRand commented 5 years ago

I suspect this is not high priority for Namecoin (as we don't have full blocks)

@domob1812 Well, it's not particularly important in terms of blocks being full, but name owners who have just accidentally issued an incorrect name update (e.g. with invalid JSON) would probably like to be able to amend the transaction without waiting for the original transaction to be mined. That was my motivation for making sure that it works well in Electrum-NMC.

domob1812 commented 5 years ago

@JeremyRand: Yeah, that certainly makes sense. I still don't think it is high priority, though, as you can just wait one block and send another update in that case. But sure - it is nice to have, I will take a look how easy that is to fix when I have time.

JeremyRand commented 5 years ago

Okay. Until it's fixed, I guess I'll leave Electrum-NMC in the current state of my RBF batching adaptations (everything seems to work properly except when the replaced transaction is a name transaction; in that case the user gets an error message and has to wait for the name transaction to be mined before they can re-issue the new transaction). RBF batching is disabled by default (for now) in upstream Electrum, so hopefully this quirk won't cause too much bother in the interim.