Skip to content

Conversation

anna-carroll
Copy link
Contributor

@anna-carroll anna-carroll commented Apr 22, 2024

Allow block data submission via 4844 blobs OR calldata

@anna-carroll anna-carroll self-assigned this Apr 22, 2024
src/Zenith.sol Outdated
/// @param s - see _submitBlock.
/// @custom:reverts see _submitBlock.
/// @custom:emits see _submitBlock.
function submitBlock(BlockHeader memory header, bytes memory blockData, uint8 v, bytes32 r, bytes32 s) external {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probablt make this bytes calldata otherwise it will get calldatacopy'd to memory and cost significant gas in memexpansion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i knowwwww, it's annoying, the reason i had it like this is because the shared _submitBlock function needs to have committedData as memory because bytes memory encodedHashes can't be calldata...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but i think i can fix this, sec

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm yeah, actually no. i think as long as we're abi-encoding the blockData into the blockCommitment it's going to calldatacopy... 🤔

Copy link
Contributor Author

@anna-carroll anna-carroll Apr 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, we could do this instead: 4aaca56

make Zenith agnostic to the submitted blockData. once we wean ourselves off emitting blockData (eg remove the BlockData event) the contracts will no longer calldatacopy the blockData :)

Copy link
Contributor Author

@anna-carroll anna-carroll Apr 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is also overall cleaner contract code, but it puts even more onus on the Node to gracefully discard any malformed, invalid or junk blockData. given that that has already been a dynamic between the Builders <> Node, this seems right to lean into.

let's align on & document the expected encoding for blockData that is considered valid by the node.

something like:
abi.encodePacked(0x01, packedBlobHashes) OR abi.encodePacked(0x02, rlpEncodedTransactions)
and some rules around how we handle missing blob hashes, invalid transactions, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per our discussion in DMs, we can also include a bytes32 blockDataHash in the function params which is also hashed in the Sequencer signature. Zenith still ignores the raw blockData, but the Node will reject the block if blockDataHash != keccak256(blockData)

this is reminding me: we need to think about how the block numbers of rollup blocks won't necessarily coincide with the sequence number on Zenith. because sequence numbers on Zenith can be consumed by invalid blocks.

Copy link
Contributor Author

@anna-carroll anna-carroll Apr 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added here: 6c7f37d

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block numbers of rollup blocks

this should be the mainnet block number

Copy link
Member

@prestwich prestwich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes this is good.

@anna-carroll anna-carroll merged commit 995e1b9 into main Apr 25, 2024
@anna-carroll anna-carroll deleted the anna/multiplex branch April 25, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants