eth: Redeems and refunds can be overwritten and lost without error.

[JoeGruffins]

Ethereum uses an account number plus nonce to prevent double spends. It is perfectly normal for a tx to be replaced by another and disappear before it is mined. In dex, this means that redeems and refunds with a low gas price may be silently replaced by the next transaction.

For example, here I have set a redemption gas cap too low to be mined. The ui will show everything is fine:

Looking at the tx from the harness:

[joe@hyrule harness-ctl]$ ./alpha attach --exec 'eth.getTransaction("0x690ae52732c282b0960ed9fbcee827e9502d64c8806a2cb0c39434d2f5a3111a")'
{                                                                                                                                                                                                                                                                               
  accessList: [],                                                                                                                       
  blockHash: null,           
  blockNumber: null,               
  chainId: "0x2a",
  from: "0x1c8f908d99f62522f8e0ad9504ffcc187e5514ca",                                                                                   
  gas: 63000,                                                                                                                           
  gasPrice: 1000000000,                            
  hash: "0x690ae52732c282b0960ed9fbcee827e9502d64c8806a2cb0c39434d2f5a3111a",
  input: "0xf4fd17f900000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001fa16bdf6e523e2a3bfbcb232ea9f4579023e2c404083137f9a3e03ea6319615b1ca0df49d8965ce5314405e43d819d02c3df33915869825072e97b72774d
ec7e",     
  maxFeePerGas: 1000000000,  
  maxPriorityFeePerGas: 1000000000,
  nonce: 16,              
  r: "0xcf2e9d9f0c440054bc27f7f2395c3d062c6a2e1fbf08a2bccff709972bb6eb77",
  s: "0x4ff989ee3495a90c237ea1b7268526a7d82db64e8588a1723033eca2fec99f7f",
  to: "0x2f68e723b8989ba1c6a9f03e42f33cb7dc9d606f",
  transactionIndex: null,
  type: "0x2",
  v: "0x0",
  value: 0
}

Then, I make another trade with a fee that can be mined:

The new init tx:

[joe@hyrule harness-ctl]$ ./alpha attach --exec 'eth.getTransaction("0x4744e0da9ada37bbaeb2c48f3c229d8a0ecf26371c624214e6733f8860ddb173")'
{
  accessList: [],
  blockHash: "0xff25a8e6b79c7b3a7df443d17dc66df78321c370902aeba2f72322a54c7acf5e",
  blockNumber: 395,
  chainId: "0x2a",
  from: "0x1c8f908d99f62522f8e0ad9504ffcc187e5514ca",
  gas: 135000,
  gasPrice: 2000000007,
  hash: "0x4744e0da9ada37bbaeb2c48f3c229d8a0ecf26371c624214e6733f8860ddb173",
  input: "0xa8793f9400000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000006201c315467667f7ea19ca8c5f0c2d2b60803e88b525430a8849d8ab83cdd509e51f
56a60000000000000000000000001c8f908d99f62522f8e0ad9504ffcc187e5514ca0000000000000000000000000000000000000000000000008ac7230489e80000",
  maxFeePerGas: 200000000000,
  maxPriorityFeePerGas: 2000000000,
  nonce: 16,
  r: "0x4848c3ea1baeab3d8afdbfa2a1a6742e758107e3d6b474ec080230b5b00f14ec",
  s: "0x72b5c445de072c67aa2c5130643d5fcf402454b920c15737f9d33a3f84927888",
  to: "0x2f68e723b8989ba1c6a9f03e42f33cb7dc9d606f",
  transactionIndex: 0,
  type: "0x2",
  v: "0x1",
  value: 10000000000000000000
}

And our last redeem:

[joe@hyrule harness-ctl]$ ./alpha attach --exec 'eth.getTransaction("0x690ae52732c282b0960ed9fbcee827e9502d64c8806a2cb0c39434d2f5a3111a")'
null

The funds appear "immature" until you restart the client, and then there is no trace they ever existed. No error or anything.

If you continue to make trades with a gas fee that is too low, you do see everything come to a stop, as the second low fee will use the next nonce and even if you replace the second tx and onward, all tx must be mined in order, so you are in another, slightly different, bad state.

I guess these tx, need to be saved and resent periodically, until they are mined, unless we are sure the fee will always be adequate?

I wonder if all coins could benefit from redeem and refund watching? There are more serious implications with eth, as the nonce must be in sync, but I would expect it's possible for other coins to have unmined and unnoticed txs? i.e. trade is not finished for client until final redeem/refund tx has a certain number of confirmations.

[chappjc]

So the second eth trade makes a transaction with the same nonce as the redeem from the first trade? Well that's a bug and a half. Is that our nonce accounting or go-ethereum's? We use leth.ApiBackend.GetPoolNonce I think so why is it reusing nonces?

I wonder if all coins could benefit from redeem and refund watching? There are more serious implications with eth, as the nonce must be in sync, but I would expect it's possible for other coins to have unmined and unnoticed txs? i.e. trade is not finished for client until final redeem/refund tx has a certain number of confirmations.

UTXO coins could have their redeem/refund txns watched, but since there's no chance of the wallet that published those from spending the contract prevout accidentally there's no issue. It's common to have chained swaps from multi-lot orders that have their Nth swap funded by the previous swap's change, which may be unconfirmed still, like you said it's not as easy to cancel/replace like with eth where you just use the same nonce.

If a funding utxo txn is never mined, it's either low fee or never propagated. So to address the propagation issue, it could be periodically rebroadcast, but that's the job of the wallet then, not dex wallet controller. Once dcrwallet/bitcoind/btcwallet is handed the transaction to broadcast, they ensure that it is continually rebroadcast until it is mined (or perhaps no longer valid). At least that's the case with current wallet software. Clearly ETH does not attempt that...

[JoeGruffins]

We use leth.ApiBackend.GetPoolNonce

I think this is only true for our nodeclient.sendTranaction, otherwise the go generated contract is handling this.

[chappjc]

I think we must be using GetPoolNonce incorrectly. Does it just return the same nonce until if finds that nonce used?
The contractor is going to be leaving the TransactOpts's Nonce nil, which signals to use pending state, so it's going to hit GetPoolNonce (internally) again, or some other nonce state.
Sounds like the solution is to set the nonce in newTxOpts, or should the nonces be handled internal to go-ethereum

[JoeGruffins]

I don't think this path uses GetPoolNonce at all. I think we only use that for withdrawals.

[chappjc]

I don't think this path uses GetPoolNonce at all. I think we only use that for withdrawals.

Right I mean internal to go-ethereum. It has to pick a nonce somehow because we don't set it in TransactOpts although we could.
See our call to (*BoundContract).Transact (from initiate) >(*BoundContract).createDynamicTx > (*BoundContract).getNonce > c.transactor.PendingNonceAt (link) I think we need to see what these nonce functions return if we call them repeatedly. Which are idempotent and which just increment the nonce?

[JoeGruffins]

Yeah, maybe handling the nonce ourselves would work? Probably the tx would get mined eventually if the fee wasn't too too low. But, using the next nonce also means that that tx cannot be mined until the previous.

[chappjc]

I hope we don't have to specify the nonce ourselves, but if that's what ends up happening in *BoundContract if we don't...

Yeah, the parent txns holding up child is a pain, and it's exactly the same thing we face with UTXOs. However if we make sure server doesn't get caught up on the txid but rather that swap state in contract (at least after initial audit), it should be easier to address that with client-initiated replacement swap txns. Yet another can of worms.

[buck54321]

abigen nonce retrieved at https://github.com/ethereum/go-ethereum/blob/6ce4670bc03602bb6638c490f892e848647c15c0/accounts/abi/bind/base.go#L352

[chappjc]

ethereum/go-ethereum#2880
ethereum/go-ethereum#15349

[chappjc]

supposedly resolved in ethereum/go-ethereum#15794 (1.8.21)
Starting to seem like we just have a logic race. Namely, we have transact calls that internally request the next nonce based on txpool and get the same nonce since neither have put a tx into the txpool, then they each broadcast, one cancelling the other. If that's correct, it suggests we have to manage the nonce at the application layer.

[JoeGruffins]

I just witnessed this with a tx that was not low fee (I guess, no record of the fee is left), but because the tx was sent close to another. Following is the evidence.

Make an initiation:

2022-02-18 11:17:14.730 [INF] CORE: Broadcasted transaction with 2 swap contracts for order 553044c99a47f52a086e7b7f3b9f13ab7d4fd74a81684ee6272dc87e16ee966e. Assigned fee rate = 200. Receipts (eth): [{ tx hash: adc5f2b558c0cdde6080d9ee83b7004851d30872eb9ac76fc2123cda7f8f2898, contract address: 0x2f68E723b8989BA1C6a9F03E42F33cb7Dc9D606f, secret hash: 6cccc343f6770e6a0744668d1d9dc12f7e79ed9b0a761c74f189496154cddf93 } { tx hash: adc5f2b558c0cdde6080d9ee83b7004851d30872eb9ac76fc2123cda7f8f2898, contract address: 0x2f68E723b8989BA1C6a9F03E42F33cb7Dc9D606f, secret hash: 5c270be7cc3c87b80fb59502e54f88f484b354ee5c8e30540fad72f3e5f16181 }].

And later the Redeems (same account/wallet):

2022-02-18 11:17:18.367 [INF] CORE: Match 5c72b83b1440bb43a0903b1d9466f57d545533104761790672a6e912f26299f3 complete: sell 1000000000 dcr
2022-02-18 11:17:18.368 [INF] CORE: Match 67975b9b58deaba0eaa7cf77eab0e4e4f7b8a148ba5260486f13cbfbbc6cb29d complete: sell 1000000000 dcr
2022-02-18 11:17:18.370 [INF] CORE: Notifying DEX 127.0.0.1:17273 of our eth swap redemption 0x2012b1da137375b1ba240e4106fe3941652c5e992a4a99b3078c33a1641cd636 for match 67975b9b58deaba0eaa7cf77eab0e4e4f7b8a148ba5260486f13cbfbbc6cb29d
2022-02-18 11:17:18.370 [DBG] CORE: notify: |POKE| (order) Match complete - Redeemed 1.021690000 ETH on order 5b031052 - Order: 5b031052bb914849759ff1ce92b8fd9e75ec0c0e5661a27ae1f261f9ca340c88
2022-02-18 11:17:18.371 [INF] CORE: Notifying DEX 127.0.0.1:17273 of our eth swap redemption 0x53bad0cb938ff7f2c1410aef87eb8ce79299f0ccfa459b69c32c9151fcc72501 for match 5c72b83b1440bb43a0903b1d9466f57d545533104761790672a6e912f26299f3

And fetching the tx afterwards, after a redemption is not found:

$ ./alpha attach --exec 'eth.getTransaction("0xadc5f2b558c0cdde6080d9ee83b7004851d30872eb9ac76fc2123cda7f8f2898")'
{
  accessList: [],
  blockHash: "0x2e95cee3d2068109872ab4f4a93814bd408fe316003314c5d24bb138cf2811eb",
  blockNumber: 42,
  chainId: "0x2a",
  from: "0x1c8f908d99f62522f8e0ad9504ffcc187e5514ca",
  gas: 248000,
  gasPrice: 2003869187,
  hash: "0xadc5f2b558c0cdde6080d9ee83b7004851d30872eb9ac76fc2123cda7f8f2898",
  input: "0xa8793f940000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000620f721c6cccc343f6770e6a0744668d1d9dc12f7e79ed9b0a761c74f189496154cddf930000000000000000000000001c8f908d99f62522f8e0ad9504ffcc187e5514ca0000000000000000000000000000000000000000000000000e2dc5a45091a00000000000000000000000000000000000000000000000000000000000620f721c5c270be7cc3c87b80fb59502e54f88f484b354ee5c8e30540fad72f3e5f161810000000000000000000000001c8f908d99f62522f8e0ad9504ffcc187e5514ca0000000000000000000000000000000000000000000000000e2dc5a45091a000",
  maxFeePerGas: 200000000000,
  maxPriorityFeePerGas: 2000000000,
  nonce: 0,
  r: "0x4d624f42e920b4bb7e27ecc47d6500527fd4f60ef4cd1d91b994ce7699c6228d",
  s: "0xc5158f19fdaaf3217a56ba48dcf5619bb320edfcf090f6ce90824daefb853a5",
  to: "0x2f68e723b8989ba1c6a9f03e42f33cb7dc9d606f",
  transactionIndex: 0,
  type: "0x2",
  v: "0x0",
  value: 2043380000000000000
}

./alpha attach --exec 'eth.getTransaction("0x2012b1da137375b1ba240e4106fe3941652c5e992a4a99b3078c33a1641cd636")'
null

$ ./alpha attach --exec 'eth.getTransaction("0x53bad0cb938ff7f2c1410aef87eb8ce79299f0ccfa459b69c32c9151fcc72501")'
{
  accessList: [],
  blockHash: "0x8686746ea7da71cc599cbd7cf8b3854a684aa5bb10f054c9b8f17583caeee577",
  blockNumber: 45,
  chainId: "0x2a",
  from: "0x1c8f908d99f62522f8e0ad9504ffcc187e5514ca",
  gas: 63000,
  gasPrice: 2002607327,
  hash: "0x53bad0cb938ff7f2c1410aef87eb8ce79299f0ccfa459b69c32c9151fcc72501",
  input: "0xf4fd17f900000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001c0ca85e1960609fc7cc544449f401928ceb0d40f4f852b48d3889db393aa79fd6cccc343f6770e6a0744668d1d9dc12f7e79ed9b0a761c74f189496154cddf93",
  maxFeePerGas: 200000000000,
  maxPriorityFeePerGas: 2000000000,
  nonce: 1,
  r: "0x9fe7ee335d849eb80d4737fc3c2327e9254d176412bae40958a7829256757340",
  s: "0x2ddc75145d550d3776fb2865dfc43927016184e448d564ed030d31a0a49c2f44",
  to: "0x2f68e723b8989ba1c6a9f03e42f33cb7dc9d606f",
  transactionIndex: 0,
  type: "0x2",
  v: "0x1",
  value: 0
}

Although 2012b1da was sent before 53bad0cb according to logs, it seems it's nonce got taken by the following transaction.

So, I guess I'll attempt to add logic to deal with the nonce ourselves?

[chappjc]

So, I guess I'll attempt to add logic to deal with the nonce ourselves?

I see no other choice. We can't leave it unset when calling the contract methods because the default apparently just reuses nonces. Sorry I haven't had time to look any closer at this.

[chappjc]

Should be resolved as long as submitting the transaction is what makes GetPoolNonce advance and not something asynchronous in the handling of the submitted transaction. Excellent testing and resolution @JoeGruffins