feat: Add block validation logic against validated transactions (in-memory)

[sergerad]

Context

We need block validation to involve a check that all the transactions in a given block have previously been validated by the Validator.

We will begin by doing this in memory. Followup PR will add persistence.

Relates to #1316.

Changes

• Update Validator transaction validation to store validated transaction headers.
• Update Validator block validation to check for previously validated headers.

[Mirko-von-Leipzig]

I suggest not manually managing the locks as it becomes easy to refactor your way into lock issues. How about replacing the lock type alias with a proper struct ValidatedTransactions instead?

#[derive(Default, Clone)]
struct ValidatedTransactions(RwLock<HashMap<TransactionId, TransactionHeader>>);

impl ValidatedTransactions {
    pub async fn check(&self, txs: impl Iterator<TransactionId>) -> Result<(), BlockValidationError>{
        let inner = self.0.read().await;

        ...
    }

    pub async fn remove(&self, txs: ...) {
        let inner = self.0.read().await;
        for tx in txs {
            inner.remove(tx);
        }
    }
}

[Mirko-von-Leipzig]

Although thinking on this some more, I don't think we can actually remove these transactions at this stage.

We should only remove transactions once we know that the block has successfully landed in the store? Right now we're assuming that if this function is called that the caller successfully completes its end i.e. we're being optimistic.

Do we not need two methods, one for signing and one for the store to submit a BlockCommitted(TransactionIDs)?

[bobbinth]

I would keep the transactions around for a long time. As I mentioned in my other comment, until we have persistence implemented, we could probably use something like an LRU cache (with fairly generous capacity) to keep transactions around. This way, we should be able to avoid the need for manual dropping.

[sergerad]

Added Lru impl

[bobbinth]

Looks good! Thank you! I left some comments inline.

[bobbinth]

I think we won't have the time to implement proper persistence for this before the next release. So, maybe this could be something like an LRU cache? We could set the size to be pretty big (e.g., thousands of transactions), but at least we'll be able to keep it bounded and we won't need to manually manage transaction eviction.

In the longer run, we probably want to store transactions for a long time for audibility purposes - but that's something to implement when we add persistence mechanism.

[sergerad]

I think we won't have the time to implement proper persistence for this before the next release

If we release the in-memory version won't we risk causing a halt if we restart the node? Which is to say that we can't release this until persistence is in place.

[Mirko-von-Leipzig]

imo just adding a validator.sqlite with a basic

CREATE TABLE transactions (
    id BLOB PRIMARY KEY,
    header BLOB NOT NULL,
    body BLOB NOT NULL,
    timestamp INTEGER NOT NULL,
);

is probably quicker to do than the time we spend thinking and discussing this :)

The "hardest" part will be the cli configuration and briefly thinking about how/if concurrent writes are a concern.

[sergerad]

There was also the issue of code factoring because the validator db wouldn't go through the store. I had a PR a while ago that implemented a basic approach for it. I assume we are still of the mind that it doesn't make sense for the validator db to be tied to the store, but I haven't thought about it for a while now.

[Mirko-von-Leipzig]

It would be its own separate database

[bobbinth]

I would keep the transactions around for a long time. As I mentioned in my other comment, until we have persistence implemented, we could probably use something like an LRU cache (with fairly generous capacity) to keep transactions around. This way, we should be able to avoid the need for manual dropping.

[bobbinth]

Looks good! Thank you! I left one small comment inline.

[bobbinth]

Let's bump this up to 10K transactions.