Keep all transactions in Batch

[ImJeremyHe]

Regarding EspressoSystems/espresso-sequencer#763

@sveitser's attempt at summarizing the change:

The main ideas

1. Arbitrum sequencer now builds a batch (aka an L1IncomingMessage with L2msg of kind L2MessageKind_Batch) with all the transactions instead of filtering out invalid ones.
2. A new kind of L2 message L2MessageKind_EspressoTx = 10 is used for espresso transactions so that they can be parsed differently than L2MessageKind_SignedTx.

During replay the messages from the sequencer to the L1 inbox are read into the wasm VM. Since all transactions are contained in the message they are now available in the wasm VM and we can validate that the transactions sequenced by hotshot are the same as the ones sent to the inbox.

During sequencing and validation the same code is used to produce blocks. Therefore a honest sequencer and validator should still produce the same results.

@ImJeremyHe's complements:
I'd like to highlight a couple of aspects in the L2's block validation process.

• instead of validating transaction root, receipt root, or block hash separately, Arbitrum validates a block by reproducing it with the same input and comparing through the block hash.
• message is a critical factor in this process, as it provides the necessary data for accurate block reproduction.
• ProduceBlockAdvanced will execute the user transactions and filter out all the invalid transactions even though a noop-hooks is set.
• ProduceBlock parses the transactions from the messages and then executes the ProduceBlockAdvanced.
• A user tx can yield other txs and they will be packed in the same block
• message is built from all the valid user transactions (making a smaller size).
• Based on the above two points, using the message and ProduceBlockAdvanced function can create the same block for validators.

For Arbitrum sequencer:
User Txs ->ProduceBlockAdvanced(Get a block here) -> User Valid Txs. -> Message -> Append the block

For Validators:
Message -> ProduceBlock -> User Valid Txs -> ProduceBlockAdvancd(Get a block here) -> Validate

In this change:
For sequencer:
Raw data from HotShot -> Message -> ProduceBlock -> User Txs (exclude the malformed txs here) -> ProduceBlockAdvanced(Get a block here) -> Append the block

For validators:
Message -> Validate The Message(todo, check the commitment) -> ProduceBlock -> User Txs (exclude the malformed txs here) -> ProduceBlockAdvanced -> Validate

A little trick here is that we don't attempt to make the message as smaller as it could. We just make the message contains all the information from HotShot and that's why we get the message at first while arbitrum get the message after creating a block.

[jbearer]

We can't just discard transactions here without checking our work in the derivation pipeline. Otherwise this centralized service has the ability to arbitrarily censor transactions that have been outputted by the Espresso sequencer.

Somehow, we need to include the list of discarded transactions in the batch metadata and check when we validate the batch that

1. The transactions in the batch interleaved with the discarded transactions make up all of the transactions in the corresponding Espresso block
2. The discarded transactions are in fact invalid

For the OP stack integration, the way we ended up doing this is by changing the op-geth block-building functionality to filter out invalid transactions and put them in a separate list. Then the validating node reconstructed the full batch of transactions by interleaving this list with the valid transactions and calling the exact same Geth endpoint to derive the rollup state.

Also, this change only handles malformed transactions. But for demo purposes, it is actually much more important to handle invalid transactions, particularly transactions with bad nonces.

I think we need to think of a more complete plan for how we will handle these transactions before we start implementing. cc @sveitser @nomaxg

[sveitser]

This is a new transaction type that indicates that the tx that follows is a tx coming from the espresso sequencer. The handling of espresso transactions by the sequencer is different than for the normal transaction in that the sequencer will not filter out malformed or otherwise invalid transactions (e. g. bad nonce).

[sveitser]

@ImJeremyHe Should this change only apply to espresso transactions but remain unchanged for other transactions?

[ImJeremyHe]

Unfortunately, I don't have a definitive answer. However, based on my understanding, encountering this situation in the arbitrum system is not expected. The message is not constructed from any malformed transaction and any challenges in deserialization might indicate a potential issue in the internal code, though I can't be certain.

[ImJeremyHe]

No worries if we run into errors during the test. In that case, I'll simply switch to another message type.

[sveitser]

This function is sequenceTransactionsWithBlockMutex but without filtering transactions according to the hooks. Or in other words, if espresso is used as a sequencer, keep all the transactions.

[sveitser]

@ImJeremyHe Suggest adding a documentation comment to the function. Also maybe call it SequenceTransactionsEspresso?

[jbearer]

Since we are not using hooks to filter invalid transactions, won't this fail if there are any? It seems like we need to use the same hooks we were using before, and also use those same hooks during replay.

This should only affect execution/computation of the state within this node, but not affect the message which gets sent to L1, because....

[sveitser]

It shouldn't fail. The current sequencer runs through ProduceBlockAdvanced and later applies the hooks to filter out transactions. So this function is intended to be called with invalid transactions and produce a block. Of course, can't say for sure if it actually behaves that way before we add a test.

nitro-espresso-integration/execution/gethexec/executionengine.go

Lines 345 to 405 in 7c4fb85

	block, receipts, err := arbos.ProduceBlockAdvanced(
	header,
	txes,
	delayedMessagesRead,
	lastBlockHeader,
	statedb,
	s.bc,
	s.bc.Config(),
	hooks,
	)
	if err != nil {
	return nil, err
	}
	blockCalcTime := time.Since(startTime)
	if len(hooks.TxErrors) != len(txes) {
	return nil, fmt.Errorf("unexpected number of error results: %v vs number of txes %v", len(hooks.TxErrors), len(txes))
	}
	if len(receipts) == 0 {
	return nil, nil
	}
	allTxsErrored := true
	for _, err := range hooks.TxErrors {
	if err == nil {
	allTxsErrored = false
	break
	}
	}
	if allTxsErrored {
	return nil, nil
	}
	msg, err := messageFromTxes(header, txes, hooks.TxErrors)
	if err != nil {
	return nil, err
	}
	msgWithMeta := arbostypes.MessageWithMetadata{
	Message: msg,
	DelayedMessagesRead: delayedMessagesRead,
	}
	pos, err := s.BlockNumberToMessageIndex(lastBlockHeader.Number.Uint64() + 1)
	if err != nil {
	return nil, err
	}
	err = s.streamer.WriteMessageFromSequencer(pos, msgWithMeta)
	if err != nil {
	return nil, err
	}
	// Only write the block after we've written the messages, so if the node dies in the middle of this,
	// it will naturally recover on startup by regenerating the missing block.
	err = s.appendBlock(block, statedb, receipts, blockCalcTime)
	if err != nil {
	return nil, err
	}
	return block, nil

[jbearer]

It looks like the hooks are being invoked somehow by ProduceBlockAdvanced, and afterward the sequencer is just processing the results (where errors for individual transactions have been stored in the hooks). So I worry that without any hooks to catch transaction errors, the whole block will fail. But maybe you're right, the best thing to do is just try it, and it's simple enough to add the hooks back in if it becomes necessary.

[jbearer]

IIUC, here we are sending msg to L1. msg contains all the transactions including invalid ones, even if some invalid transactions were filtered out by the hooks in ProduceBlock(Advanced)?

[sveitser]

That's why the SequenceMessage (I think we should call it SequenceTransactionsEspresso instead) function was added. It's doing what the sequencer was doing but not applying the hooks after producing a block.

[sveitser]

ProduceBlockAdvanced will execute the user transactions and filter out all the invalid transactions even though a noop-hooks is set.

@ImJeremyHe Here by "filter" do you mean "ignore"?

[ImJeremyHe]

ProduceBlockAdvanced will execute the user transactions and filter out all the invalid transactions even though a noop-hooks is set.

@ImJeremyHe Here by "filter" do you mean "ignore"?

Yes. Here ProduceBlockAdvance executes the tx and if err is returned, it will be ignored by message creating process and by the block

[sveitser]

LGTM

[jbearer]

It seems the only difference between this and the regular L2MessageKind_SignedTx above is that the Espresso txs are encoded as JSON and the regular transactions are encoded as binary. Why not just use the binary encoding for Espresso transactions and then we don't need separate parsing for them?

[nomaxg]

Having custom parsing for espresso txns might be the move - realized today that the justification I added to the message header isn't actually to the batch (seems obvious in retrospect), so we may need to encode the justification in the l2 message.

[jbearer]

I don't understand. We may need to do some special parsing and have a special message type for the batch as a whole, but why do we need separate parsing for the individual transactions, when the only difference from normal transaction parsing is that we use JSON instead of a binary format? Further, I'm not even sure it is correct to use JSON here as is, based on my other comment below

[nomaxg]

Ah yes you're right, I agree, we should only need a special message type for the batch as a whole

[ImJeremyHe]

Yes, you are right. I didn't realize that we should do the parsing in a totally different type.
I am not sure what is your question about JSON fomrat. We make a forwarding when receving the user transactions here. And user txs is encoded in JSON before sending to espresso sequencer.

[jbearer]

I see. I don't think we should be using JSON there either. JSON is fine for encoding the Espresso transaction, but when we encode the Arbitrum transaction here, I think we should use MarshalBinary, which should use the standard RLP encoding that you would get if, e.g. you submitted a transaction via the usual web3 APIs. We gain nothing and waste a lot of data by keeping EVM transactions JSON-encoded throughout the entire stack.

However, since we are currently being consistent in using JSON everywhere, it seems that this doesn't need to block this PR. We can merge it as-is and switch to a binary encoding later. @ImJeremyHe can you create an issue to follow up on this?

[nomaxg]

LGTM