Improve mempool eviction policy

[arc0035]

I'm using Smapp 1.2.9 on Mac. Steps to produce:
1)I create a fully new address with no gas in it, then I build a spawn transaction to test my sdk. the transaction is broadcast successfully to the node and the SubmitTransactionResponse tells me that the state is in mempool
However, the transaction is still in the mempool and seems not removed.
2) I transfer some smidge to this account ,after the tx is applied, the transaction is still in the mempool, neither removed or mined.
3) I try to build another spawn tx, but its nonce from projected state is still 0, causing this tx fails, reporting:
"Failed to verify transaction: tx already exists"
4) Even if I use TransactionState api to check this transaction, it still show state "TRANSACTION_STATE_MEMPOOL"
.I think if the transaction does not have enough fund to pay balance/gas, it should be removed immediately and reporting states like "TRANSACTION_STATE_REJECTED"

Is this expected behavior? How to bypass it?

[lrettig]

I've seen some similar behavior, not sure if it's the same issue or different:

• generate and broadcast a tx that's valid aside from having an invalid counter (e.g., higher counter value already consumed)
• tx gets mined into block (example: https://explorer.spacemesh.io/txs/0x2dee9b43ac885b2b013b93fa115ee36877d92935aef80c23d47d2821cfa765cd), likely because optimistic filtering wasn't working at the time - but isn't effective
• tx continues to sit in mempool; API reports TRANSACTION_STATE_MEMPOOL. at present this tx will never leave the mempool.

There are definitely many such transactions in the mempool today. But I don't think they're blocking following txs.

[lrettig]

I'm beginning to understand what's going on here - although I'm also beginning to suspect that these are two separate (possibly related?) issues and that we might need more information to troubleshoot OP's issue.

The issue as I understand it is that transactions at present can only have three possible internal states in the node:

go-spacemesh/common/types/transaction.go

Lines 123 to 134 in 92b7762

	// TXState describes the state of a transaction.
	type TXState uint32
	const (
	// PENDING represents the state when a transaction is syntactically valid, but its nonce and
	// the principal's ability to cover gas have not been verified yet.
	PENDING TXState = iota
	// MEMPOOL represents the state when a transaction is in mempool.
	MEMPOOL
	// APPLIED represents the state when a transaction is applied to the state.
	APPLIED
	)

We may want to add an INEFFECTIVE, IGNORED, DROPPED, EXCLUDED_FROM_MEMPOOL etc. state.

As soon as a tx has been parsed (its header has been read, i.e., it's syntactically valid), it's assigned state MEMPOOL:

go-spacemesh/sql/transactions/transactions.go

Lines 171 to 177 in 92b7762

	state := types.PENDING
	layer := types.LayerID(uint32(stmt.ColumnInt64(2)))
	if layer != 0 {
	state = types.APPLIED
	} else if parsed.TxHeader != nil {
	state = types.MEMPOOL
	}

This doesn't mean it's contextually valid, e.g., its counter may be wrong. It also doesn't mean the tx is actually in the mempool. It may only be in the database -- and I guess we save all txs in the database, even ones that will never be effective. So there's also a question of whether we should drop these txs from the database, and on what logic. (Even a tx that appears that it'll never be effective because, e.g., a tx from the same principal with a higher counter has already been processed, could theoretically appear in a valid older block that a node missed, or due to a reorg.)

[brusherru]

This issue is still actual and should be solved.
I've published some transactions which cannot be processed (except maybe the first one):

1. I've published self-spawn with nonce 0, but had zero balance
2. When I trying to publish again it returns "tx already exist"
3. So I've published a couple of self spawn transactions with other nonces (going out of order, e.g. 2, 100, 22222)

All of them has mempool status, does not change account nonce, and blocks from submitting tx (with the same nonce).
The status is not changing over the time.

And if they actually persist in a mempool, then it might be an attack vector.

Here is one of my transactions:

curl -d "{\"txid\":[\"Qdke0v/mYF5XZSuFQAKlTvQir2PpViEqHq6i8YtCttI=\"], \"include_state\": true, \"include_result\": true,\"limit\":1 }" -X POST https://wallet-api.spacemesh.network/spacemesh.v2alpha1.TransactionService/List

{"transactions":[{"tx":{"id":"Qdke0v/mYF5XZSuFQAKlTvQir2PpViEqHq6i8YtCttI=","principal":"sm1qqqqqq89p5d8jxyyrp0jgfvfldcdu4d8vpgnclcqcvr47","template":"sm1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqg56ypy7","method":0,"nonce":{"counter":"2222"},"maxGas":"100432","gasPrice":"1","maxSpend":"0","raw":"AAAAAADlDRp5GIQYXyQliftw3lWnYFE8fwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAG5IgTMkR8HdG1+KGb+Mclb5PwOdyHXo5ING3vu66DFeG63YoNkH5Vwri7llqIyQf5rwJCG/SSdPP6IBf4/gLFmMz1zPRo+DABnFO9+uXOCBK+UTu3RqSUzbohqOOAmwRL9rQc=","type":"TRANSACTION_TYPE_UNSPECIFIED","contents":{"singleSigSpawn":{"pubkey":"cc911f07746d7e2866fe31c95be4fc0e7721d7a3920d1b7beeeba0c5786eb762"}}},"txResult":null,"txState":"TRANSACTION_STATE_MEMPOOL"}],"total":1}%        

[brusherru]

Okay, I run another test on standalone network and published 2 self spawn transactions (with nonces 0 and 1) with zero balance. Once I got positive balance this transactions were applied, one processed and one rejected.
So I guess they are actually in the mempool...

curl -d "{\"txid\":[\"X9YwBi7EoA5g5Ro7rfhsKzvzsgiYM99rE6v8GYB5XA0=\"], \"include_state\": true, \"include_result\": true,\"limit\":1 }" -X POST http://127.0.0.1:8080/127.0.0.1:9095/spacemesh.v2alpha1.TransactionService/List

{"transactions":[{"tx":{"id":"X9YwBi7EoA5g5Ro7rfhsKzvzsgiYM99rE6v8GYB5XA0=", "principal":"standalone1qqqqqq89p5d8jxyyrp0jgfvfldcdu4d8vpgnclcwe3cnj", "template":"standalone1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqg6me6zj", "method":0, "nonce":{"counter":"1"}, "maxGas":"100432", "gasPrice":"1", "maxSpend":"0", "raw":"AAAAAADlDRp5GIQYXyQliftw3lWnYFE8fwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEEBMyRHwd0bX4oZv4xyVvk/A53Idejkg0be+7roMV4brdiEw511ZDZXJFeLIQCcg66tiFDbllS0aA1jjvKRzg0iwO3BMNq9t36BNc/ff6DI39UlFT5++fFHRrg7NKQua6DAQ==", "type":"TRANSACTION_TYPE_UNSPECIFIED", "contents":{"singleSigSpawn":{"pubkey":"cc911f07746d7e2866fe31c95be4fc0e7721d7a3920d1b7beeeba0c5786eb762"}}}, "txResult":{"status":"TRANSACTION_STATUS_FAILURE", "message":"account already spawned", "gasConsumed":"100432", "fee":"100432", "block":"CUezL8kHOkimH00ipFJSwlnQOvE=", "layer":22, "touchedAddresses":["standalone1qqqqqq89p5d8jxyyrp0jgfvfldcdu4d8vpgnclcwe3cnj"]}, "txState":"TRANSACTION_STATE_PROCESSED"}], "total":1}%                                


curl -d "{\"txid\":[\"mpPfyNOSyXdtdBkGMPK7R3pY4zBzaDoQv9s2NuCfhZs=\"], \"include_state\": true, \"include_result\": true,\"limit\":1 }" -X POST http://127.0.0.1:8080/127.0.0.1:9095/spacemesh.v2alpha1.TransactionService/List

{"transactions":[{"tx":{"id":"mpPfyNOSyXdtdBkGMPK7R3pY4zBzaDoQv9s2NuCfhZs=", "principal":"standalone1qqqqqq89p5d8jxyyrp0jgfvfldcdu4d8vpgnclcwe3cnj", "template":"standalone1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqg6me6zj", "method":0, "nonce":{"counter":"0"}, "maxGas":"100432", "gasPrice":"1", "maxSpend":"0", "raw":"AAAAAADlDRp5GIQYXyQliftw3lWnYFE8fwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEABMyRHwd0bX4oZv4xyVvk/A53Idejkg0be+7roMV4brdimN8FJS5OKvkrGE9r5HK5JQYGeFDYnBwr1Yfl52zMbdGVbJriA2dOuIkgjV3Kyh99BjqrGEQbitZcyMZ9TMhuAA==", "type":"TRANSACTION_TYPE_UNSPECIFIED", "contents":{"singleSigSpawn":{"pubkey":"cc911f07746d7e2866fe31c95be4fc0e7721d7a3920d1b7beeeba0c5786eb762"}}}, "txResult":{"status":"TRANSACTION_STATUS_SUCCESS", "message":"", "gasConsumed":"100432", "fee":"100432", "block":"CUezL8kHOkimH00ipFJSwlnQOvE=", "layer":22, "touchedAddresses":["standalone1qqqqqq89p5d8jxyyrp0jgfvfldcdu4d8vpgnclcwe3cnj"]}, "txState":"TRANSACTION_STATE_PROCESSED"}], "total":1}

[lrettig]

A few points. First off, there are three related but distinct issues/questions here:

• the first involves the API, and the transaction states it returns. We should not discuss that here. CC @kacpersaw
• the second is a UX/wallet question. It involves how the wallet monitors a user's submitted transactions, how it becomes aware of when a transaction is mined or dropped from the mempool, and the tx state/status/journey. It also involves the UX around RBF (replace by fee). We should also not discuss that here.
• the third involves how the mempool in go-spacemesh works: what's the policy for inserting, updating, and dropping transactions. We should discuss this here.

It seems like our goals for the third are:

• make sure we understand the mempool admission/eviction procedure, and make sure it's clear, and clearly documented. We should adopt a similar strategy to Bitcoin: set a fixed mempool size, allow the node operator to change this, be liberal with what we admit up to this limit, begin to drop txs with a low likelihood of inclusion once the mempool is full.
• we should prioritize txs by fee. This applies both to duplicate txs from the same principal (to allow RBF), and also to a full mempool (drop lower fee-paying txs in favor of higher fee-paying txs).
• handling the situation where an invalid tx blocks one or more valid txs (from the same principal, with higher nonces) is tricky. Really, this is not the responsibility of the node. It's the responsibility of the wallet/user. The user should not submit a tx that will never be valid, followed by a higher-nonce tx that is valid. And the wallet should allow the user to speed up or cancel a tx using RBF. Even if wallets prevent this, however, a user might encounter it if using multiple wallets. There's a potential DoS vector here if the node tries to get "too clever," e.g., by evaluating all txs from a given principal for validity before proposing/including them in a block. We should do something simple here: likely, wait maybe a single block, and then evict all txs that are still invalid. I need to do some more homework and see what, e.g., go-ethereum does.

Roughly speaking, once a user submits a tx to a node, these are the possible outcomes:

• tx is admitted to the mempool and has a MEMPOOL state.
• tx is mined into a block and has a CONFIRMED state (in the API this is currently split into MESH and PROCESSED).
• tx is rejected upon admission. No txid is generated.
• tx is initially admitted to the mempool but later evicted (due to RBF, or a full mempool). In this case, when the user queries the tx state via the API they should receive "tx unknown."

Am I missing anything?

I'll do a little more homework into how other nodes/protocols handle this.

[brusherru]

One another case:

• I've published spend tx with amount higher than I have and Nonce 19,
• Then I've published another spend tx with proper amount and the same Nonce 19,
• First transaction is still in the mempool, but I expect it to become rejected because of the Nonce.

On TestNet 13.
The tx that stuck in the mempool:

curl -d "{\"txid\":[\"4H3xp37Pq2YHDxiN6VVqwO6TDDC1VV6Q4SsXopkkiYU=\"], \"include_state\": true, \"include_result\": true,\"limit\":1 }" -X POST https://testnet-13-api.spacemesh.network/spacemesh.v2alpha1.TransactionService/List

{"transactions":[{"tx":{"id":"4H3xp37Pq2YHDxiN6VVqwO6TDDC1VV6Q4SsXopkkiYU=", "principal":"stest1qqqqqq89p5d8jxyyrp0jgfvfldcdu4d8vpgnclcad4mm8", "template":"stest1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqgf0ae28", "method":16, "nonce":{"counter":"19"}, "maxGas":"36218", "gasPrice":"1", "maxSpend":"99999999000000000", "raw":"AAAAAADlDRp5GIQYXyQliftw3lWnYFE8f0BMBAAAAADlDRp5GIQYXyQliftw3lWnYFE8fxMANu8heEVjAY+ppGvPx1hjTo0KGySkuhRvQc1iNVHkrxPg5yDouhH4vz5zhJdHilglCTpSVyEt0j/qHU7K3AD+vNPMKjcYVAI=", "type":"TRANSACTION_TYPE_SINGLE_SIG_SEND", "contents":{"send":{"destination":"stest1qqqqqq89p5d8jxyyrp0jgfvfldcdu4d8vpgnclcad4mm8", "amount":"99999999000000000"}}}, "txResult":null, "txState":"TRANSACTION_STATE_MEMPOOL"}]}

Processed one:
https://testnet-13-explorer.spacemesh.network/txs/0xb1ce27d3ffd99b2ddeb99b4d8f7d70851fdf93246f132d2a22a0a7c47e56ad18

[lrettig]

I spoke to this in my previous comment. This is an antipattern and wallets shouldn't allow this - you should not be able to submit a spend for more than your account contains. If you do, the correct thing to do is RBF with a valid transaction.

Having said that, we still need to understand how other nodes/protocols handle this and we may just want to evict bad txs after some time, as I also said in my previous comment.

[pigmej]

Completely agree. On the Wallet side we should definitely add validation for "not possible to send more than the current balance".

We definitely should do RBF, just because people can do mistakes. And one of the easiest thing to do when you want to cancel, is to replace it with transaction to yourself.

But additionally on top of that I think we need some auto expiry for mempool transactions. But it's my gut feeling not an educated suggestion.

[acud]

@pigmej @lrettig I've added a PR #6164 to add a unit test for RBF which appears to be already possible so we can at least strike that one off the list (at least to the best of my understanding). As for the rest of the issues described above I'll have another look tomorrow.

[lrettig]

That's good news. Thanks for taking a look.

[brusherru]

Btw, another inconsistency in the behavior:

• if I send spend tx with insufficient funds — it gets stuck in mempool
• if I send drain tx trying to drain more funds than vault stores — it gets rejected with corresponding error message: https://testnet-13-explorer.spacemesh.network/txs/0x17ff1a80ca6d173bbb972b429d760a8d205403e6b47ee08f1ce1b7e5e4429315

I believe either both txs should be rejected or both should be accepted into the mempool.

[acud]

Most of the issues here should be solved with the merging of #6185. @brusherru If you have other suggestions we could handle them in future issues.