@roman-khimov just put some doubt in my mind in a discussion here: https://github.com/neo-project/proposals/issues/97#issuecomment-715309816
It seems to me that the logical operation of Verification is to occur before Invocation, since we may use these witnesses inside the Invocation. And, it seems logical to me that, if we are using CheckWitness, we assume this witness is correct (returns True), right?
Ok then. Maybe, if we are verifying all transactions in a pack, and only then, executing all the invocations (I don't remember exactly if this is the procedure), this can cause inconsistencies and even break witness assumptions.
Suppose some witness depends on checking the signature of some owned contract (written in storage), and if we change this owner, we have to suppose that the next transactions would fail this verification, regardless of being in same block or not. Or are we only taking into account the states in the block BEFORE execution...
If verification fails, then tx must FAULT invocation. And that's even more important to have Post Verification flag stored on the chain, as recently proposed, otherwise we risk breaking witness and verification invariant, which are sacred.
If this is the case, I think we should definitely change this behavior, by re-executing Verification before every execution of Invocation (including side-effects of previous tx in same block), issuing FAULT if witnesses not satisfied, for security reasons.
Can you clarify @shargon?
Or are we only taking into account the states in the block BEFORE execution...
Currently yes, if the transaction was chosen for the block, we doesn't check again if the verification it's valid or not.
If your verification depends to the state, and two transactions enter, the next one can be wrong verified and enter as success.
I think that this behavior is logic because the CN doesn't execute the TX, otherwise you will pay the fee for unverified transaction.
Thanks a lot for the explanation @shargon , but really, we need to fix that. It is not acceptable that some invalid witness is considered for valid execution, I guess you can imagine why.
The fix is simple though, and it doesn't affect pricing strategy:
This allows for one transaction to take into account the side effects of other transactions. This fix is valid for both Neo 2 and Neo 3, and it is an important one, as storage reads seem to be quite useful in verification, and Neo 2 strongly relies on them.
I think that this behavior is logic because the CN doesn't execute the TX, otherwise you will pay the fee for unverified transaction
I understand your perspective, in which "logical" means, the way it is makes some sense... but paying for unverified transaction, in worst case scenario, is better than having some CheckWitness returning inconsistent result. User tx will have opportunity to be "pruned" by mempool, or just before speaker puts in block, but it may happen to be paid and unused (otherwise we drop precious space from blocks). The good thing is that Invocation itself will not be paid (and could even be refunded, but only doable in Neo3).
- first, in mempool, we verify transactions up-front, using GAS quota strategy or any other
- then, primary node selects transactions to be put on block (and re-verifies them, because we don't trust our own mempool!)
- finally, before executing the Invocation part for every tx, we execute the Verification (once again!), and if it fails, we don't even execute Invocation (it FAULTS).
I think we can trust our own mempool and not do the second verification, but even if we're to eliminate this step, reverifying before execution would seriously affect TPS metrics. We can skip standard (state-independent) verification scripts, though.
This allows for one transaction to take into account the side effects of other transactions.
But overall all of that reminds me of #1285.
We can skip standard (state-independent) verification scripts, though.
This a very good point @roman-khimov . I'll take s look at the issue you mentioned.
Anyway, even if we skip state independent witnesses, we will pay some cost to detect them. And from a present / future perspective, maybe @shargon will also agree that we want nearly all, or maybe all, transactions to include balance state verification.
I'm positive that such impact is not big on tps, but even if it is, I think we must guarantee CheckWitness invariant at all costs.
After some thoughts and discussions, I may have changed my mind, so this is how I see :
Neo2:
Neo3
If thats the case, I guess we can close this, thanks fir all clarifications