feat(challenger): added stacked preconfs in the same slot

[merklefruit]

Overview

This PR adds the logic to handle an important missing piece of the challenger logic: stacked preconfirmations.

A couple useful definitions:

• commitmentReceiver: this is the EOA that requested the preconfirmation. it is the sender of the signed transaction that has been preconfirmed by a proposer.
• commitmentSigner: the party that signed the commitment. This is anyone running a Bolt sidecar, aka proposers.

The problem

The issue, in short, can be summarized by the following example:

• Alice requests 3 preconfirmations for the same slot from the same proposer
• The proposer accepts all of them and signs them, but then includes only 1 and 2, skipping 3.
• Alice would try to open a challenge, only to see that their nonce was TOO_HIGH, because the challenger contract has no way to prove that transactions 1 and 2 were preconfirmed, as the challenge was for a single committed transaction.

The solution

To address this, we now require the challenge to be opened with an array of committed transactions.
These transactions cannot be random though, they must follow these criteria:

• they must be sent by the same entity: a "stack" is valid only for each individual EOA, as we really only care about the sender's nonce and balance in order to validate the protocol rules for inclusion
• they must be signed by the same bolt proposer, which we recover through EC recovery
• they must target the same slot, which is part of all signed commitments
• they must have strictly sequential (monotonically increasing) nonces

Then, when it comes to verifying their inclusion, we check each transaction's nonce one after the other to validate them, before performing the usual merkle inclusion proof.

Currently the challenge is considered BREACHED if any of the commitments inside the bundle is breached.
A further improvement could keep track of the number of breached commitments easily, but that's out of scope here.

How it works

The main change this PR introduces is a new structure for the Challenge and Proofs structs:

    struct Challenge {
        bytes32 id;
        uint48 openedAt;
        ChallengeStatus status;
        uint256 targetSlot; // new
        address challenger;
        address commitmentSigner;
        address commitmentReceiver; // new
        TransactionData[] committedTxs; // previously this was not an array
    }

• commitmentReceiver explained above
• targetSlot refers to the slot contained in all the commitments of this challenge
• committedTxs contains data about each individual committed transaction, like its nonce and gas limit.

    struct Proof {
        uint256 inclusionBlockNumber; // same as before, prefixed with "inclusion" for clarity
        bytes previousBlockHeaderRLP; // block against which we check initial nonce and balance
        bytes inclusionBlockHeaderRLP; // block against which we check committed transaction inclusion
        bytes accountMerkleProof;
        bytes[] txMerkleProofs; // this is now an array, must have length equal to Challenge.committedTxs
        uint256[] txIndexesInBlock; // same as above
    }

Meta

• updated challenger interface and implementation
• updated existing tests
• updated documentation

[thedevbirb]

The logic is very clear and that's great. Left some observations.

[thedevbirb]

Can you add a test also for stacked preconfirmations?

[merklefruit]

done!

[mempirate]

Why was this renamed? I think CHALLENGE_BOND is more clear since it's the bond required to open a challenge

[merklefruit]

see above comment by Lore: #239 (comment)

[mempirate]

Do we clear expired challenges somewhere?

[merklefruit]

No, they live in the storage forever, as we rely on this as an oracle for understanding if a slashing request is legit.

[thedevbirb]

Just wondering: after a challenge has been resolved can we keep its ID in the challengeIDs storage but remove it from challenges? We don't need that data anymore and can also give some gas refunds if I understood correctly (https://www.evm.codes/?fork=cancun#55).

With this, then we can perform this check on _resolve:

    function _resolve(bytes32 challengeID, bytes32 trustedPreviousBlockHash, Proof calldata proof) internal {
        if (!challengeIDs.contains(challengeID)) {
            revert ChallengeDoesNotExist();
        }

		// This means the challenge has been resolved and deleted from storage successfully.
        if ( challenges[challengeID].id == bytes32(0)) {
            revert ChallengeAlreadyResolved();
        }

		// ...

[merklefruit]

Let me make a test to see how much gas this would save

[merklefruit]

Before: BoltChallengerTest:testResolveChallengeFullDefenseSingleTx() (gas: 697302)
After: BoltChallengerTest:testResolveChallengeFullDefenseSingleTx() (gas: 554812)

Savings: 142.490 gas which at ~30 gwei would cost like $10 at current ETH prices.
This will be even more for N constraints. I think overall it makes sense and I'll think of a better solution
to keep track of the basic necessary data and get rid of everything else!

[merklefruit]

I'm not 100% sure this works well - TBD

[merklefruit]

this leaves an edge case open:

• proposer commits to 10 transactions (same sender, same slot)
• proposer includes only the first 3, the last 7 are broken commitments
• problem: challengeID should be unique for that sender and slot, but the challenge could be opened with any amount of signed commitments as calldata
• we can't guarantee that the first time the challenge is opened it will contain the full list of broken commitments
• it would be currently possible to open the challenge with commitments (1, 2, 3, 4), or (1, 2, 3, 4, 5), etc until (1, 2, 3, 4, 5, 6, 7, 8, 9, 10) and all of them would be considered lost because there is at least a broken commitment in each.

A granular solution would be to keep track of all the tx hashes in storage in a mapping of (bytes32 -> bool) and slashing will be done at most once per tx hash for which the mapping is true. Thinking of possible alternatives.

[thedevbirb]

I've seen that now the signature is computing by concatenating signatures which is good but still leaves this edge case exposed. I remember the latest proposal on it is to create a challenge per commitment breached, but I don't see this logic in the contract otherwise the function _computeChallengeID would return bytes32[] memory challengeIDs. Is there any reason we're not doing it?

[merklefruit]

As discussed privately, this will be addressed in a later iteration. tracked in #248