Validate a new account state before attempting to store it

[igamigo]

What should be done?

When a transaction is executed and its results are applied in the store, it's possible that the account is not modified at all. This means that the nonce remains unchanged, as do the storage slots and the vault assets. If an account was new (nonce=0), and such a transaction is executed, when the transaction results are applied, the account gets saved with the same nonce and no seed which is rejected by the store. We should catch this before applying the result to more gracefully surface the error.

How should it be done?

Before transaction results get applied, check that there is no account state with the same nonce and error otherwise.

When is this task done?

When we error before applying transaction results with an account's existing nonce.

Additional context

An example of this happening is running cargo run --release init -p 1 1 1 1 on this repo/branch

[igamigo]

This issue is a bit stale after some recent validation changes. I think what we would need to do is make sure the account state does not already exist in the database. That is, we could check for one of these two:

• (AccountId, Nonce) pair does not already exist in the store
• account_hash does not exist in the store

I think we should do the latter and return a new ClientError variant if it already exists. Additionally I think we can set the account_hash as the primary key instead of the (AccountId, Nonce) pair as it is right now. Tagging @bobbinth to validate.

[bobbinth]

I think we can set the account_hash as the primary key instead of the (AccountId, Nonce) pair as it is right now.

I think this should be fine. I don't remember if there was any motivation for using (AccountId, Nonce) as the primary key - but it seems like using account_hash is a better approach.

Thinking about the overall issue, I think we have a bit more complexity here to deal with than what I originally thought. Specifically, it may happen that we'll end up with multiple stale account states. For example:

1. We submit a transaction which updates the account state to X. This will result in a new record being created in the accounts table in a pending state. But let's say the transaction does not go through for some reason (i.e., the network drops it).
2. A different client managing the same account submits a transaction which updates the account to state Y, and that transaction goes through. Assuming we are dealing with a public account, we'll see a new account state appear on chain - but it will not be consistent with our local DB. So, we'll need to update the local DB accordingly a. if the account is private, the account becomes effectively locked - i.e., we know there is a new state, but we don't know it and so cannot issue any transactions against this account. We should probably add a separate issue for handling this case.

Another interesting case to handle: if our transaction doesn't go through, at some point, we'll need to assume that it is canceled (I think one interesting protocol-level feature to add could "self destructing transactions" - i.e., if a transaction doesn't make it into the chain within the next 5 mins, it will not make it into the chain at all). We need to somehow remove (or update) the now-stale account state so that we can submit another transaction against this account.

[igamigo]

Another interesting case to handle: if our transaction doesn't go through, at some point, we'll need to assume that it is canceled (I think one interesting protocol-level feature to add could "self destructing transactions" - i.e., if a transaction doesn't make it into the chain within the next 5 mins, it will not make it into the chain at all). We need to somehow remove (or update) the now-stale account state so that we can submit another transaction against this account.

Essentially rolling back a transaction locally would just involve "removing" the account row from the transaction that caused it and disabling output notes + re-enabling notes (unless some were spent in which case maybe the node can send back some useful info). This should be fairly simple to implement at least superficially, there could be a combination of these conditions where it might not be as straightforward to resolve.

A different client managing the same account submits a transaction which updates the account to state Y, and that transaction goes through. Assuming we are dealing with a public account, we'll see a new account state appear on chain - but it will not be consistent with our local DB. So, we'll need to update the local DB accordingly a. if the account is private, the account becomes effectively locked - i.e., we know there is a new state, but we don't know it and so cannot issue any transactions against this account. We should probably add a separate issue for handling this case.

I believe we discussed both of these at some point. I think we are at a point where we can handle the first one gracefully: The node returns that the account state is invalid so we can detect the error and ask for the new one. For the second one, I don't think there's much we can do to handle it, right? As the state is private the user would be responsible and the node couldn't do much more than return a pertinent error.

[bobbinth]

For the second one, I don't think there's much we can do to handle it, right? As the state is private the user would be responsible and the node couldn't do much more than return a pertinent error.

Yes, agreed - the one thing we should do here is mark the account as "locked" somehow (maybe introduce a new status) - so that the users (e.g., CLI) could handle it properly.