Incompatibility between sync<>conservative cache<>vm API

[dshulyak]

problem

• sync downloads layers in parallel with verifying/applying them to the state
• syntactic validity of the transaction can't be verified if previous layer is not yet applied to the state (more precisely if spawn wasn't applied to the state)
• parse_payload is a part of syntactic validity
• verify is a part of syntactic validity

so when conservative cache receives synced transaction there is a high chance that account from that transaction is not yet spawned, and the transaction won't be parsed. and sync won't work in general.

possible solutions

initialize template from self-spawn stored in conservative cache

not sure if it will be possible to find spawned accounts without executing transaction.
very likely it won't be possible if spawn will be a part of the unrelated method defined in wasm.

can be used as a short term solution, but probably not future proof.

store full header in the transaction (e.g. remove parse_payload)

doesn't enable Verify method, so still not possible to verify syntactic validity.

doing this solves the problem only partially, not worse it.

applying blocks during sync should not go through conservative cache

conservative cache needs parsed header to accept transaction into the cache.

• sync downloads blocks and transactions without validation
• thread that applies transactions should Verify as well
• if one of the transactions fails verification - block is invalid and should be removed from the node

how does it change flow for blocks that were built in the conservative cache?
looks like a big change in conservative cache. would prefer to avoid

[dshulyak]

cc @noamnelke @countvonzero

[noamnelke]

Regarding step 2: principal should be known for any transaction, even without context, but maybe it's not worth parsing it - not sure. In theory we can even run ParsePayload for most transactions, unless you're syncing all of history and there's a huge gap between what you download and what you've applied. If there's only a few layers difference then I assume that very few transactions will not have a spawned account yet. However, it might be simpler to just treat all transactions the same way.

Steps 3-5: I think it might simplify the implementation (and the mental model) if we do a data availability step before starting to execute the block. Parse all unparsed transactions, try to re-fetch bad ones (verifying them immediately) and erroring at this stage if we can't obtain and verify some transaction. Once this step is successful we just do the normal block execution.

Step 6 should be removed. The conservative state only describes the diff over the actual state due to unapplied transactions. Any transaction that is being applied to the actual state, doesn't need to be applied to the conservative state.

Instead of step 6, once all blocks have been applied then we should apply the remaining transactions (those that were referenced in blocks that shouldn't be applied, in proposals, or in the mempool, assuming we're syncing those) to the conservative state.

---

When pruning invalid blocks, the transactions in them don't necessarily need to be pruned. If they're valid they should be added to the mempool (assuming they're not already in later blocks / proposals). Add this comment to the issue you're creating.

[dshulyak]

principal should be known for any transaction

it should be known, but it makes parsing even uglier than it is now

Step 6 should be removed.

it can't be removed, conservative cache needs to be updated with applied nonce. otherwise it will invalidate valid gossiped transactions

[noamnelke]

it can't be removed, conservative cache needs to be updated with applied nonce. otherwise it will invalidate valid gossiped transactions

That's not correct. Conservative state should be a diff on top of actual state. If a nonce is consumed in the actual state it doesn't need to be in the diff.

[countvonzero]

i think i get it. i'll take a stab at the implementation.

[dshulyak]

Just to clarify, i don't know what is a part of conservative state and what is just a regular mempool management. It is all in one module, so i call everything conservative state as i don't see a boundaries. Regarding the solution for the problem above i am sure that you need to maintain consistency of the mempool regardless of how transaction was received. full change is available in

go-spacemesh/txs/conservative_state.go

Lines 212 to 232 in b7ee989

	// txs without header were saved by syncer without validation
	if mtx.TxHeader == nil {
	req := cs.vmState.Validation(mtx.RawTx)
	header, err := req.Parse()
	if err != nil {
	return nil, nil, fmt.Errorf("parsing %s: %w", mtx.ID, err)
	}
	if !req.Verify() {
	return nil, nil, fmt.Errorf("applying block %s with invalid tx %s", toApply.ID(), mtx.ID)
	}
	mtx.TxHeader = header
	// updating header also updates principal/nonce indexes
	if err := cs.tp.AddHeader(mtx.ID, header); err != nil {
	return nil, nil, err
	}
	// restore cache consistency (e.g nonce/balance) so that gossiped
	// transactions can be added succesfully
	if err := cs.cache.Add(&mtx.Transaction, mtx.Received, nil); err != nil {
	return nil, nil, err
	}
	}

[countvonzero]

update from slack conversation:

[kimmy lin]
another approach i want to try, is to have the txs go through conservative cache (instead of going directly to db). that way, maybe i can cache the spawn tx from the previous block and will be able to parse the spend tx successfully

[Dmitry Shulyak]
the problems that i couldn't figure out with this:

• if there is a collision for principal, how do we know which spawn tx to use for a template
• if spawn can be used programmatically..
  the 2nd one is definitely not genesis

[kimmy lin]
reg. collision for principal's spawn tx, don't we simply compare the gas price then break tie with timestamp?
but then each node will choose different spawn tx if the gas price is the same
wait no. if we are syncing a block to be applied, then we know for sure which spawn tx to use.

[Dmitry Shulyak]
maybe it is possible to choose it deterministically. if we have 2 txs that spawn same principal - can we be sure that the first one will succeed?

wait no. if we are syncing a block to be applied, then we know for sure which spawn tx to use.

i don't think so. they may be even on different layers

[kimmy lin]
but as long as we know the input vector, we know exactly what block to apply for each layer, then we know which spawn tx to use. all the unapplied txs will stay in the conservative cache until applied (edited)

[Dmitry Shulyak]
for example tx A from layer 11, and tx B from layer 12 spawn same principal but with different template. naturally tx B will fail if tx A is applied succesfully, but if tx A failed - then tx B will be applied
so which template to use for validation if that principal created spend tx C on layer 13

[kimmy lin]
ahhh... you are saying while in the cache, we still don't know which one will be applied successfully....
i am working on the input vector now. i'd still like to give this a try after i get the input vector right. in the mean time, i guess we'll live with inconsistent db now....

[Noam Nelke]
[@kimmy]
I think we should keep it simple for genesis and not try to apply spawns before they are in an applied block. I think this is possible, but it’s nuanced and I prefer not to complicate things for now.

[Dmitry Shulyak]
it would actually simplify code a lot and make it more robust. but unlike you i think it is impossible 🙂

[Noam Nelke]
I think you/Kimmy think it’s simple because you’re not thinking of how to handle all the edge cases. Kimmy possibly doesn’t see the edge cases, so she thinks it’s simple and possible. You probably see the edge cases, but not the possible solutions, so you think it’s impossible.
I am not 100% certain that this is possible, but I have ideas. Making it possible will make it complex, anyway, so I think we should wait and then later consider if it’s worth the complexity.

[kimmy lin]
honestly the only goal i have at this point is to always save txs in DB with correct header. this means to parse/verify before saving to db.
now that we don't consider two spawn txs with the same principal, please list the edge cases you can think of now so i can translate them into test cases. i promise i won't spend too much time on it 😄

[Noam Nelke]
Let’s see where my suggestion doesn’t achieve this goal (to be able to parse every transaction before storing it).
We’re supposed to reject gossip transactions that we can’t parse, so that’s good.
We’re supposed to reject block proposals that contain transactions that we can’t parse - and then we don’t store these transactions either, so still good!
The only case we must store a transaction that we can’t parse is if it’s included in a block. Regardless of the decision to vote against the block or just abstain. Regardless of whether or not we implement tentative spawning from the mempool - there’s always the option that a spawn transaction was never seen and then a spend transaction is included in a block that we need to handle.
So as I see it, tentative spawning doesn’t alleviate the need to implement unparsed transaction storage. I can’t think of anything that makes that redundant. But it is expected to be rare.
Tentative spawning (and eventually tentative deployment, when we support deploying templates) is something I believe can work, but it requires marking the tentative template and account as such so that out of order followup transactions are handled in a consistent way (so regardless of when you execute the state transitions - you end up ignoring / executing the same transactions). We should implement this, but we should do it after genesis, to keep things simpler for now.
Thoughts,
[@kimmy]

[kimmy lin]
The only case we must store a transaction that we can’t parse is if it’s included in a block. Regardless of the decision to vote against the block or just abstain. Regardless of whether or not we implement tentative spawning from the mempool - there’s always the option that a spawn transaction was never seen and then a spend transaction is included in a block that we need to handle.
thank you for such thoughtful answer. yes. that's exactly what's implemented now by Dmitry. and what you described about tentative template assignment/deployment is what i had in mind previously.
additionally i was thinking, if we can delay saving those unverifiable txs in a synced block until we are ready to execute it, then we don't have to save any unparsed txs to db at all.
when we fail to parse a tx, we fail to execute it, we don't save it. combined with execution result (where we record reasons why a tx fails), i think this is possible/doable.

[Noam Nelke]
If I correctly interpret what you mean, then this is an implementation detail. If the transaction is available to us (it must be if we know that it’s un-parsable) then we should retain it. We can store it in a different database, different table, or whatever, but what we shouldn’t do is discard it. If a block includes a transaction that is un-parsable at execution time we know we shouldn’t execute it, but we must retain the transaction, even if it’s just to prove that it’s un-parsable and shouldn’t be executed. If we discard it, there’s no way to prove that.
It’s important to note that even if we have the data to be able to parse the transaction and we could theoretically execute it, if the spawn transaction wasn’t executed by the time the transaction is supposed to be executed we should NOT execute the transaction (because there’s no proven consensus that it’s executable).
Perhaps this realization will reduce your motivation to always store the header with the transaction in the DB. Why? Because before fetching a transaction from the database and executing it, we have to evaluate if it’s parsable only based on applied state. If we manage to obtain a header for a transaction not based on applied state, then we have to check if the spawn was executed before executing the transaction. If we do this check anyway, then we only need to ever look at headers if we can have them without trickery. I hope this explanation is clear, lmk otherwise.

[Dmitry Shulyak]
vm intentionally always parses header from the raw transaction when it is applied. it is cheap, and simplifies other things
the stored header is necessary only for api and mempool, the alternative was to always parse it when raw tx is loaded from database - but it required a lot of changes so that i decided not to do that
i want to double check another edge case. if transaction wasn't parsable in the past (e.g. we couldn't even get nonce) - but became parsable in the future. is it executable in this case? if yes, does it make sense to store a result for the one that wasn't parsable in the past?

[Noam Nelke]
about always parsing in the VM - great.
About your question: what matters is the global state at the time of block execution. If a transaction cannot be parsed based only on the global state (applied transactions) at the time the block is executed and then later the spawn transaction is applied (in a later block) then we don’t go back and apply old transactions in previous blocks.
If you mean that an older block that wasn’t applied becomes valid in the future, essentially retroactively applying the spawn transaction in the past - then this is a re-org. Everything that was applied after that layer needs to be reverted and blocks re-applied, so at that point a transaction that couldn’t be parsed originally can get applied. This is another reason we must retain un-parsable transactions, even though I expect this case to be very rare.

[Dmitry Shulyak]
not exactly
for example there is block A in layer 10 that had tx with nonce 1 (ignore how it got included there). we execute this block and can't parse this transaction. we probably will create a tx result/receipt for this transaction
later block B in layer 11 has a self-spawn tx, that executes succesfully
later block C includes same tx from block A, but now it can be executed successfully.

[Noam Nelke]
I see, I would phrase this as “does a transaction that can’t be parsed get consumed, so it can’t be included in another block again?” - is that what you meant?

[Dmitry Shulyak]
idk, in this case it is not consumed (e.g. we don't charge gas, nonce is not consumed as well) (edited)

[Noam Nelke]
I would say the answer is no. We should just ignore it, same as a transaction that doesn’t pass verify(). The difference is only that a transaction that failed verify() will do so consistently forever, but an un-parsable transaction may become parsable.
Regardless of what we call it (I agree with your objection to “consumed”), retaining the information that the transaction has been included in a block in the past means we need a special mechanism (normally the nonce is enough, but here we’d need a growing list of IDs) and I don’t think it serves any purpose. I.e. if I’m a miner and I just want to include spam in a block - I can change a bit in the transaction and it’ll have the same effect, different ID and verify doesn’t get called for this anyway. For that matter I can just include arbitrary bytes.

[Noam Nelke]
bottom line, I think we should just ignore these transactions completely, which means that they might be included and executed in the future.

[countvonzero]

based on the above conversation, i am not making this issue a top priority to fix.