"Conflicts" transaction attribute type

[roman-khimov]

Proposed attribute This attribute makes the chain only accept one transaction of the two conflicting and adds an ability to give a priority to any of the two if needed. It's useful for solving such problems as:

• replay attack mentioned in #1502 User sending transactions like in https://github.com/neo-project/neo/issues/1502#issuecomment-602752663 would add "Conflicts Tx1" attribute to Tx2, "Conflicts Tx1, Conflicts Tx2" to Tx3 and "Conflicts Tx1, Conflicts Tx2, Conflicts Tx3" to Tx4, no matter which one is going to be relayed and added to the chain any of the other couldn't be reused because of the conflict
• overriding erroneously sent transaction There is a short time window between transaction relaying and transaction acceptance, in some cases it could be used to cancel the transaction before it's accepted, to do that one sends new transaction that conflicts with the previous one, but has a higher network fee.
• specifically creating pairs of transactions of which only one could be accepted This is going to leveraged by P2P signature collection service from #1573.

The attribute has Uint256 data inside of it containing the hash of conflicting transaction. It is allowed to have multiple attributes of this type.

During transaction processing additional actions are to be done for transactions containing attributes of this type:

• check for conflicting transaction in the ledger if conflicting transaction is already on-chain, the transaction is rejected as invalid
• check for conflicting transaction in the mempool if conflicting transaction is present in the mempool, then two transactions are compared for their network fees, the one with a bigger network fee wins, so the other one is either rejected as invalid (if it's not in the mempool) or evicted from the mempool and replaced by the new one
• reverification follows the same basic rules, any conflicting transaction on-chain makes mempooled one invalid
• persistence logic should check in-block transactions for this attribute and store dummy conflicting transactions in the DB to prevent conflicting ones from being accepted

Since this attribute incurs some additional processing overhead it may have an additional network fee.

Neo Version

• Neo 3

Where in the software does this update applies to?

• Ledger
• P2P (TCP)

[roman-khimov]

There is a prototype now in nspcc-dev/neo-go#1507.

The logic has been adjusted a bit to solve an attack possible with the following scenario: Alice signs transaction A and sends it to the network, then Bob sees this transaction in the mempool and immediately creates transaction B conflicting with A and paying higher fee. The original logic described here allowed to accept B in favor of A and if Bob has enough GAS he can effectively DoS Alice for as long as he wants to. So B has to be signed by Alice to prevent this, if it's not then B is invalid and A stays in the pool irrespective of B's fee.

And we also effectively reserved version 255 for dummy conflicting transactions to check for conflicts/presence in the DB with one DB access (it can be done without that, but for now it's simpler this way).

[igormcoelho]

I think this is an interesting problem... instead of attributes, I'd always prefer to keep standard features, such as cosigners. If you add a script on verification like "Neo.Transaction.Exists txid", and it also considers txs in mempool at that time, this may solve your problem. The issue is that txs become stateful again, as in your proposal. A stateful tx with mempool interference may also interfere in consensus: imagine one node prefers txA and other txB... speaker proposes txA, but other nodes fail to extract and verify txA from p2p mempool because they already have txB. A solution may be some sort of "force receive tx" even if it seems invalid (on mempool), as during block verification it will be valid. Other possibility is to only allow this interop consider block context, not mempool, so speaker may have both on mempool, and only when assigning one by one to block it will discover that txB cannot be selected because txA was already selected (and many other variants of this). Again, I like the power of such mechanism, but we must assure it doesn't affect consensus process in a bad way.

[roman-khimov]

add a script on verification like "Neo.Transaction.Exists txid", and it also considers txs in mempool at that time

This would mean that we have to reverify any non-standard script of any mempooled transaction for any transaction added to the pool.

And functionally it only allows for transaction B to be invalid in presence of A, which is not enough, we need preemption guarantees if some conditions are met (like this doesn't allow to override any already sent but not yet accepted into block transaction).

speaker proposes txA, but other nodes fail to extract and verify txA from p2p mempool because they already have txB

Backup CNs will request txA from their peers and this transaction doesn't have to go through the mempool, so if it'll manage to arrive in time it's not a problem (at least for neo-go, but IIRC C# is also fine wrt this in the master branch).

[igormcoelho]

This would mean that we have to reverify any non-standard script of any mempooled transaction for any transaction added to the pool.

This is the same as current, will not increase number of validations: only new transactions would be validated, and previously validated are kept assumed as validated. Example: txA enters mempool (validation ok!); txB tries to enter mempool (validation fails because txA exists). From what you mentioned, some txC enters mempool, that is fine (no conflicts) with txA; but maybe txA is against existence of txC (not symmetric), is that the case? So, yes, in this case, both txA and txC would be on mempool, and only when puting to block, txA would "realize" that txC is also a candidate, thus being rejected (if txC has entered before). That's a side effect of any stateful mechanism.. and that's why transactions are re-validated before puting on block.

[roman-khimov]

some txC enters mempool, that is fine (no conflicts) with txA; but maybe txA is against existence of txC (not symmetric)

Consider txA and txB, txB does Neo.Transaction.Exists txA check, txA does no checks at all (and we can't really put Neo.Transaction.Exists txB there). Now if there is txA in the mempool and we receive txB it'd run a witness check and fail it, txB is rejected, everything is fine. But if we're in the opposite scenario where there is txB in the mempool and we receive txA, it'd be happily accepted with txB still being present in the mempool.

and only when puting to block, txA would "realize" that txC is also a candidate, thus being rejected (if txC has entered before).

Back to txA and txB from above, even if you're to do this block-level check, block B1{txA, txB} would be considered invalid, but block B2{txB, txA} is perfectly valid again, although it shouldn't be.

All of this is not a problem when using Conflicts attribute.

and that's why transactions are re-validated before puting on block.

Except they're not as we've discussed in #2017. Fees are being checked, but not witnesses.

[igormcoelho]

Fees are being checked, but not witnesses.

I hadn't realized that... so my final argument there may not be correct. For me, all witnesses are being rechecked in batch-mode before tx invocations. Anyway, if tx witness is currently stateless, there's really no need to re-check (but this proposal makes them stateful, so recheck is needed during block proposal/acceptance).

I agree there are many challenging points here, because I'd much prefer having standard cosigners checks doing the job, than a new attribute for the same behavior. I'll try to better understand the expected behavior in your proposal, to try to sketch the same without a new attribute.

[roman-khimov]

but this proposal makes them stateful, so recheck is needed during block proposal/acceptance

We just need to check for conflicts (kinda similar to UTXO double spend check) and it's feasible, already implemented in neo-go.

I'd much prefer having standard cosigners checks doing the job, than a new attribute for the same behavior.

That was one of the main conflicts to resolve for #1573, whether to leverage some witness script powers or to add attributes. Given the constraints we have I don't think it's possible to do that via witness scripts (and especially given that we don't want to parse scripts for some data we need), thus we've resorted to attributes and they're working fine for now. Though if you manage to do that it'd be interesting, of course.

[igormcoelho]

So, I think we may have a general strategy / pattern to follow in these situations... since transaction validation is intended to be stateless, we may resort to attributes in order to execute these stateful computation, but in quite constrained scenario. It's interesting, because we could build the dependency graph and perform these checks on verification, but doing that "manually" via attributes look much easier (since they are done on C#/Go/...), at the cost of increasing software complexity (one more attribute). The same situation happened with ValidUntil field, since it's also easily implementable as cosigner, but in the end, it was preferred as a transaction field.

So, with this strategy in mind, I agree with you, it's better to have it as attribute then as cosigner script.