stacks-network
deploying a contract with a duplicate name does not cause a transaction rejection
If I try to deploy a smart contract with a duplicate name to the Stacks node, it accepts the transaction but never sends it to the mempool (instead of rejecting it).
For example, if I have a contract with id STB44HYPYAT2BB2QE513NSP81HTMYWBJP02HPGK6.counter deployed and I try to deploy another one with the exact same name, I get a successful transaction response when I do so (with a valid tx_id) instead of a transaction rejection, even though the transaction will never get picked up and will never be sent to observers. This makes it impossible for users to know if/why their transaction was rejected causing confusion and/or nonce issues.
Steps To Reproduce
- Deploy a contract, wait for confirmation
- Attempt to deploy a new contract with the exact same name as the previous contract
Expected behavior
The node rejects the 2nd transaction with a rejection response to /v2/transactions
There are variations of this issue where there are multiple un-confirmed transaction in the mempool attempting to deploy a contract with the same name.
- Situation 1
- Tx with nonce M deploying contract "foo" is confirmed
- Tx with nonce N (N > M) deploying contract "foo" is submitted
- Situation 2
- Tx with nonce M deploying contract "foo" is submitted
- Tx with nonce N (N > M) deploying contract "foo" is submitted
- Tx M is confirmed
- Situation 3
- Tx with nonce N (N > M) deploying contract "foo" is submitted
- Tx with nonce M deploying contract "foo" is submitted
- Tx M is confirmed
- Situation 4
- Tx with nonce M deploying contract "foo" is submitted
- Tx with nonce N (N > M) deploying contract "foo" is submitted
- Tx M is included in a block with an error, due to a problem in the contract
- Tx N is confirmed
Because of this complexity, I think it is not a good idea to handle this case in the mempool admit logic. I would prefer for the node to always include the deploy transaction in the block, even if a contract with the same name has already been deployed. It is better to include the transaction, making it clear to the user what went wrong. The downside of this is that the user's fee has now been taken, but that is caused by a user error, and not by any outside forces, so this seems like a reasonable thing to do.
Because of this complexity, I think it is not a good idea to handle this case in the mempool admit logic. I would prefer for the node to always include the deploy transaction in the block, even if a contract with the same name has already been deployed. It is better to include the transaction, making it clear to the user what went wrong.
Completely agree -- failed contract calls, etc. are today included in blocks (though the result isn't, sadly, so it's up to the node to process the transaction and come to that conclusion) -- and I don't think contract deploys should be any different. Minimize the special cases :sweat_smile:
The downside of this is that the user's fee has now been taken, but that is caused by a user error, and not by any outside forces, so this seems like a reasonable thing to do.
I believe that this, in general, is the overall mindset in most of the node's code, i.e. users are charged for failed transactions as well (contract-calls, etc.). I also think is completely reasonable as you've used network bandwidth, compute and storage -- if nothing else, as a DoS-deterrent.
Because of this complexity, I think it is not a good idea to handle this case in the mempool admit logic. I would prefer for the node to always include the deploy transaction in the block, even if a contract with the same name has already been deployed. It is better to include the transaction, making it clear to the user what went wrong. The downside of this is that the user's fee has now been taken, but that is caused by a user error, and not by any outside forces, so this seems like a reasonable thing to do.
On top of eating the user's money, it also increases the size of the blockchain to include transactions that could never be valid.
I'm of the opinion that if it's possible to detect and reject a transaction before block inclusion, then the code should do so, even if it means that the mempool admission logic is more complicated as a result. The worst-case outcome of complicated admission logic is that the user doesn't get informed about the rejection via the expected channel (e.g. the RPC endpoint), but as this bug report shows, the cause of transaction rejection is ultimately knowable.
That said, I don't think the RPC endpoint is the right place for determining the totality of transaction rejection reasons, because it's not always possible to do so at the point of transaction submission. To see why, you could imagine more contrived scenarios, such as submitting two different deployment transactions for the same smart contract name via different nodes. How would the two different nodes even be aware of the other's conflicting transaction? I think instead we need to have some kind of polling interface whereby a node that mock-mines can at least tell you whether or not the transaction was considered, and if so, whether or not it was rejected (and why). I think something like this might even already exist.
EDIT: wanted to add that in general, I don't think that code anywhere should be punishing users for their mistakes if it can be avoided. We care about the users' well-being, so if we can help them avoid a common pitfall by exerting a little bit more effort, it's worth our while to do so.
I also think is completely reasonable as you've used network bandwidth, compute and storage -- if nothing else, as a DoS-deterrent.
Determining whether or not a smart contract name is already taken is extremely cheap.
Completely agree -- failed contract calls, etc. are today included in blocks (though the result isn't, sadly, so it's up to the node to process the transaction and come to that conclusion) -- and I don't think contract deploys should be any different. Minimize the special cases 😅
Contract-call failures are not the same thing. It's not actually possible to determine ahead-of-time what the contract-call and its post-conditions will evaluate to, since it can depend on information that will only be known at the time the block is produced (e.g. the VRF state, the Bitcoin hash, etc.).
Contract-call failures are not the same thing. It's not actually possible to determine ahead-of-time what the contract-call and its post-conditions will evaluate to, since it can depend on information that will only be known at the time the block is produced (e.g. the VRF state, the Bitcoin hash, etc.).
Fair enough -- but is there a DoS vector here? Do nodes/the mempool have logic to temporary-ban users repeatedly submitting a deploy-contract with the same name, or at least a form of rate-limiting?
Fair enough -- but is there a DoS vector here? Do nodes/the mempool have logic to temporary-ban users repeatedly submitting a deploy-contract with the same name, or at least a form of rate-limiting?
Yes -- you have to submit a transaction whose tx fee is above the minimum fee (and your account has to have that), and you can only have at most 25 pending transactions from your account in the mempool. This applies to all transactions of all kinds.