[HackerOne-2310746/2311934] Verify the transmissions fetched in batch proposals.

[raychu86]

Motivation

This PR performs transmission verification on transmissions fetched from batch proposals. Previously we were blindly accepting them into the batch, which could lead to possible halting cases, where malicious nodes inject faulty transactions.

This approach should should be taken instead of #3083 because it doesn't falsely eliminate valid transmissions that come in subsequent blocks. The thread in #3083 has more details.

NOTE: This puts transaction verification on the "critical path" which means we will have to verify newly seen transactions before processing batch proposals. Previously we would verify the transactions in check_next_block after the block was created, but this will slow down the block production via the BFT. We are basically front-loading the TX verification tasks now.

This should fix:
#2990
#2991

[howardwu]

We should start with a PR that just filters for !transaction.is_fee() and tag it on HackerOne-2311934.

[ljedrz]

I agree with #3098 (comment), LGTM otherwise

[randomsleep]

check_transaction_basic is also too strict for checking BatchPropose. It's possible that other validators may reject an honest validator's BatchPropose because they have different latest states. The check should be independent from the latest ledger state and ensure BatchPropose made by the honest validator won't be rejected.

If possible, I can make a PR for this.

[ghostant-1017]

check_transaction_basic is also too strict for checking BatchPropose. It's possible that other validators may reject an honest validator's BatchPropose because they have different latest states. The check should be independent from the latest ledger state and ensure BatchPropose made by the honest validator won't be rejected.

If possible, I can make a PR for this.

I think it's fine to reject the BatchPropose from an honest validator if they have different states. It means the node is behind the quorum of validators and the node will accept the BatchPropose once it catches up.

[ghostant-1017]

I think a better solution to solve it without regard to code changes:

1. Allow illegal Transaction in BatchPropose or BatchCertificate.
2. During the commit_subdag phase, Transactions are checked and executed one by one, and the result is used to decide whether to penalize the corresponding Validator( i.e: a transaction contains a Record already spent before GC round).
3. We can remove check_transaction_basic in many places, and use financial penalty to make validator not pack illegal Transaction in the BatchPropose.

[randomsleep]

I think it's fine to reject the BatchPropose from an honest validator if they have different states. It means the node is behind the quorum of validators and the node will accept the BatchPropose once it catches up.

The other nodes can be in a newer round and will never accept the BatchPropose. This will introduce liveness issues and may introduce more attacks.

[raychu86]

This should be replaced by AleoNet/snarkVM#2376 and #3104.

[raychu86]

Closing due to AleoNet/snarkVM#2376 and #3104 being merged.