search

namecoin / namecoin-core

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

Replace `queuerawtransaction` with `send` for name registrations/renewals? #552

Open JeremyRand opened 2 weeks ago

JeremyRand commented 2 weeks ago

I was looking at the implementation of the send RPC method, specifically what happens when add_to_wallet is true and the transaction isn't eligible for the mempool.

As best I can tell, this sequence of events happens:

  1. send() calls FinishTransaction(). https://github.com/bitcoin/bitcoin/blob/a38603456e9a8448874106c56966f3defb41574a/src/wallet/rpc/spend.cpp#L1310
  2. FinishTransaction() calls pwallet->CommitTransaction(): https://github.com/bitcoin/bitcoin/blob/a38603456e9a8448874106c56966f3defb41574a/src/wallet/rpc/spend.cpp#L131C13-L131C39
  3. CommitTransaction() calls SubmitTxMemoryPoolAndRelay(): https://github.com/bitcoin/bitcoin/blob/a38603456e9a8448874106c56966f3defb41574a/src/wallet/wallet.cpp#L2351C10-L2351C36
  4. SubmitTxMemoryPoolAndRelay calls chain().broadcastTransaction(): https://github.com/bitcoin/bitcoin/blob/a38603456e9a8448874106c56966f3defb41574a/src/wallet/wallet.cpp#L2044C16-L2044C44
  5. broadcastTransaction() calls BroadcastTransaction(): https://github.com/bitcoin/bitcoin/blob/a38603456e9a8448874106c56966f3defb41574a/src/node/interfaces.cpp#L686
  6. BroadcastTransaction() calls node.chainman->ProcessTransaction(): https://github.com/bitcoin/bitcoin/blob/a38603456e9a8448874106c56966f3defb41574a/src/node/transaction.cpp#L83C48-L83C81
  7. ProcessTransaction() calls AcceptToMemoryPool(): https://github.com/bitcoin/bitcoin/blob/a38603456e9a8448874106c56966f3defb41574a/src/validation.cpp#L4594C19-L4594C37

AcceptToMemoryPool also happens to be what queuerawtransaction() currently uses to test whether transaction are eligible, and chain().broadcastTransaction also happens to be what CWallet::EffectTransactionQueue currently uses to broadcast queued transactions, and SubmitTxMemoryPoolAndRelay also happens to be what MaybeResendWalletTxs uses for transactions that are in the wallet but aren't confirmed. So... it looks like we should be able to just use the send() method, without needing to maintain our own transaction queue?

Observations:

  1. MaybeResendWalletTxs only tries to resubmit every 12-24 hours for privacy reasons. This seems OK for queued renewals. For queued registrations, we could work around it by adding an extra flag that makes it try every block only for name_firstupdate transactions. Alternatively, if we're worried about bunching together renewals in a way that reveals common ownership, we could make it try every block for all name transactions.
  2. Using the wallet's built-in rebroadcasting should automatically avoid accidentally causing double-spends or address reuse.
  3. Using the wallet's built-in rebroadcasting should result in the queued transactions automatically showing up in the GUI.
  4. And of course, using the wallet's built-in rebroadcasting should yield a much smaller diff against Bitcoin Core.

Is my understanding of the code correct? Is there any reason we're not doing it this way?

JeremyRand commented 2 weeks ago

@domob1812 @yanmaani thoughts?