cartesi / rollups-contracts

Smart Contracts for Cartesi Rollups
https://cartesi.github.io/rollups-contracts/
Apache License 2.0
17 stars 37 forks source link

Raise `PayloadTooLarge` error instead of `InputTooLarge` error in InputBox #224

Closed ZzzzHui closed 5 months ago

ZzzzHui commented 6 months ago

📚 Context

Going forward, inputs are blockchain-agnostic blobs containing metadata and the input payload itself. If the blob is larger than the limit set by Cartesi Virtual Machine, contract InputBox raises an InputTooLarge error, with the input blob size and the limit, defined in the interface IInputBox. However, this error can be confusing to the users as they may not know about the construction of the input blob under the hood, and it takes some figuring out and guess work to know how to adjust their payload argument to avoid the error. The direct way would be to prompt the PayloadTooLarge error with the payload size and limit.

✔️ Solution

Replace InputTooLarge error with PayloadTooLarge. May add a constant INPUT_PAYLOAD_MAX_SIZE in the common/CanonicalMachine library.

guidanoli commented 6 months ago

How would you calculate INPUT_PAYLOAD_MAX_SIZE based on INPUT_MAX_SIZE at compile time?

ZzzzHui commented 6 months ago

After this PR being merged, we add block.chainid and address(app) to the current structure of the input blob. So the input blob structure becomes:

Field # bytes
selector 4
block.chainid 32
address(app) 32
msg.sender 32
block.number 32
block.timestamp 32
index 32
offset 32
size 32
payload DYNAMIC

Therefore, the payload size limit is

INPUT_PAYLOAD_MAX_SIZE = 
                        (INPUT_MAX_SIZE - (4 + 32 * 8))
                                                        / 32 * 32

This is based on the encoding of EvmAdvance, which might change in the future. So the better place for this constant is probably in the InputBox contract rather than CanonicalMachine library. Because if the EvmAdvance encoding changes, the InputBox contract changes anyway.

What do you think?

guidanoli commented 6 months ago

Your thought process is solid, but the code that calculates INPUT_PAYLOAD_MAX_SIZE does not take the EvmAdvance function into consideration, which means that if someone changes the signature of this function but not this equation, they will be out-of-sync.

ZzzzHui commented 6 months ago

If someone changes EvmAdvance, they will update the contract InputBox as well, which is where the constant will be in. If the equation is not updated properly, unit tests would catch that immediately.

guidanoli commented 6 months ago

Ideally, the constant should be updated automatically when the EvmAdvance function is updated, but I believe that is not currently possible with the Solidity compiler.

guidanoli commented 6 months ago

Having said that, I would keep things the way they are now.