stacks-core icon indicating copy to clipboard operation
stacks-core copied to clipboard

duplicate contract id gets stuck in mempool instead of failing

Open pseudozach opened this issue 1 year ago • 2 comments

Describe the bug When an account tries to deploy the same contract identifier again, the tx gets stuck in the mempool instead of failing immediately.

This transaction should fail, which would allow the others to be processed in sequence, but since it's stuck pending then the others following it are pending too. https://explorer.stacks.co/txid/0x8b2bc97d921d25f35c15c5ac7fc262d3b54159b2ffe2f99b593b5e463ef81535?chain=testnet You could replace that tx with nonce 56 with something like a small STX transfer and a higher fee to get it executed, but the core issue remains - that tx should've failed after the one before it landed.

Steps To Reproduce

  1. deploy a contract - wait for it to be included in a block
  2. deploy the same contract id again
  3. the 2nd tx gets stuck in mempool for 24h ~ 256 blocks even though it's invalid and will not be mined.

Expected behavior 2nd tx that tries to deploy an already existing contract identifier should fail immediately. Due to this stuck tx, user's all other pending txns are stuck until offending tx is cleared from mempool.

Additional context Here's contract already deployed with nonce=55 Screen Shot 2022-09-06 at 4 47 39 PM

And here's user trying to deploy a contract with same name with nonce=56 Screen Shot 2022-09-06 at 4 47 29 PM

pseudozach avatar Sep 06 '22 23:09 pseudozach

Hi @pseudozach

the 2nd tx gets stuck in mempool for 24h ~ 256 blocks even though it's invalid and will not be mined.

A couple things to unpack here:

  1. The tx is not invalid. It's just not mineable on the canonical fork. However, it could be mined on a different fork, should one arise later. Given that the mempool must accept transactions before it knows which forks will exist (indeed, miners use the mempool to build forks), the fact that this tx is accepted is not a bug.

  2. That said, the mempool already tries to reject transactions that are unlikely to be mined on the canonical fork. In particular it will check to see if a smart contract tx is trying to instantiate a smart contract under an already-used name, and if so, the tx will be rejected. However, there is an unavoidable TOCTOU condition with this heuristic: the canonical fork can change in-between two tx submissions, so it's possible that two txs can be accepted into the mempool that instantiate the same contract. It's unlikely, but impossible to prevent outright.

As you point out, the remedy here is to send a different transaction with the same nonce as the one that's stuck, but with a slightly higher fee, in order to RBF it.

2nd tx that tries to deploy an already existing contract identifier should fail immediately.

The Stacks blockchain will not allow miners to create blocks that include obviously-unmineable transactions. In particular, a block with a tx that tries to instantiate a smart contract whose name is already used on that fork will cause the block to be rejected. The reason for this is that it's not fair to users -- a user shouldn't have to pay a tx fee for a tx that obviously won't do what it's supposed to do once mined. Instead, the Stacks blockchain saves them their STX by prohibiting miners from including such txs.

jcnelson avatar Sep 09 '22 14:09 jcnelson

thanks for the detailed explanation, so if I understand correctly:

  • Normally node sw should've dropped this tx due to mempool logic because it's not mineable.
  • because of some potential race condition, maybe nodes kept it just to see which one gets included in the block
  • scenario is not entirely unexpected

Workaround is clear as well, I just wanted to understand if there was a justification for the current behavior which makes sense now. Issue can be closed.

pseudozach avatar Sep 09 '22 22:09 pseudozach