Store notification merkleRoot in TransactionState

[shargon]

Close #2034

Note: We can use MPT (Like in StateService)

[AnnaShaleva]

I see one potential problem with keeping the notifications or their Merkle root in the native Ledger contract (correct me, if I'm wrong):

Problem description

One of the goals that #2034 is trying to solve is aligning all side-effects of transaction execution across the network nodes, which means ensuring that after block acceptance we need to ensure that a) contracts storage has the same set of changes and b) transaction notifications are the same within all the nodes (that's pretty enough for aligning). The problem is that currently, even with StateService plugin acting as a replacement to the StateRootInHeader option, we exclude native Ledger's state from the MPT: https://github.com/neo-project/neo-modules/blob/4f55458bda29a0e80258add2a3768dcc014ab24d/src/StateService/StatePlugin.cs#L97.

This change was firstly implemented in neo-project/neo-modules#470. There are several reasons why we do that, but the most significant are described in neo-project/neo-modules#520 (although this issue is related to the StatesDumper plugin, but the idea is similar).

So here with notifications or their Merkle root, our desire is to include them into comparable (and verifiable) stateroot, but it's not enough to place them into TransactionState structure.

Possible solutions
One of the solution I was thinking about is store notifications or their root under a special prefix (not inside the TransactionState structure) and slightly modify StateService plugin to exclude only a part of native Ledger's storage items from the Merkle trie (exclude transactions and blocks, and keep notifications). But I'd like to pay @roman-khimov's attention to this problem, because there might be some problems with porting this solution to the NeoGo, because we have a different storage scheme and do not store anything in our native Ledger contract. Although it might not be a problem if we're about to store only notifications root, it takes only 32 bytes, so it likely won't exploit NeoGo's storage.

Another solution that came to my mind is to store notifications in some other contract.

Side note
The described problem will remain relevant even after (or if) we'll move from the StateService plugin to StateRootInHeader option. But maybe I'm missing something and we don't have this problem at all.

[shargon]

If we use just the hash of the storage changes in the header, we can ensure that the state will be the same, also with the notifications, the problem will be with statePlugin, it won't include the ledger data, included the notifications, for me is solved with the state root changes in the header.

[AnnaShaleva]

The problem is that StateRootInHeader doesn’t include the whole set of changes from the native Ledger’s storage; and at least it has to exclude blocks and transactions states. And thus, the problem remains the same for StateRootInHeader.

[shargon]

Yes, if we use a simple hash we will include all of them, included the Ledger changes, you can remove blocks later, but it will take the previous state hash + the current changes hash

[AnnaShaleva]

if we use a simple hash we will include all of them

Then we need to discuss it, because I don't think we need to include blocks and transactions into state root at all, even if we store them in native Ledger without witnesses. Why do we need them to be included? We have all accepted blocks and transactions relayed onto P2P level, this mechanism allows nodes to sync the chain without going deep into contract's storage level. I consider we don't need exactly the same blocks/transactions data to be included into native contract storage. E.g. in NeoGo we don't store them in native Ledger, we store them in the DB which allows us to implement some optimizations. And NeoGo's StateRootInHeader implementation (nspcc-dev/neo-go#1500, nspcc-dev/neo-go#1701) also doesn't take into account blocks and transactions states.

I know that NeoGo is not a reference implementation, and if we decide to include blocks/transactions into state, then NeoGo will follow, technically it's possible. But do we really need them to be presented into states? I'd suggest to wait for Roman's opinion on this topic.

[roman-khimov]

1. We need a complete state root in the header (How to sign the stateroot？ #1526, yay). Exchanging just the changes state root limits our abilities. Complete state root is what makes P2P state exchange #2373 possible as well as proofs.
2. This state root should capture all side-effects (Native contract to store application logs #2034).
3. Ledger state can't be included in full state root calculation.
4. So storing in the Ledger doesn't seem to be appropriate. At least unless we want to have more magic in https://github.com/neo-project/neo-modules/blob/4f55458bda29a0e80258add2a3768dcc014ab24d/src/StateService/StatePlugin.cs#L97
5. So we either move it out to a separate contract (a bit cleaner to me), or we do magic.

[roman-khimov]

Either way, this can be discussed after #2914 release.

[vncoelho]

What is current status, @shargon?

At least some basic hash of block notifications is enough. That is what some protocols do.

[shargon]

What is current status, @shargon?

At least some basic hash of block notifications is enough. That is what some protocols do.

We can continue with it