Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plasma: default commitment type conflicts with derivation version #65

Closed
tuxcanfly opened this issue Feb 26, 2024 · 12 comments
Closed

plasma: default commitment type conflicts with derivation version #65

tuxcanfly opened this issue Feb 26, 2024 · 12 comments

Comments

@tuxcanfly
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Currently the specs for plasma describe the commitment type as 0 for keccak256 commitment of the data.

Commitments are encoded as commitment_type_byte ++ commitment_bytes, where commitment_bytes depends
on the commitment_type_byte where [0, 128) are reserved for official implementations:

commitment_type commitment
0 keccak256(tx_payload)

There is small but unfortunate chance of mis-interpreting the commitment type as the derivation version due to both being calldata, for example if a sequencer node enables plasma but disables it later. Since configuration for plasma is part of the --plasma cli flags and not part of rollup.json, the likelihoood of this is high especially during fork migrations.

Describe the solution you'd like

Please consider removing the ambiguity between the derivation version and commitment type by using distinct sentinel bytes.

Describe alternatives you've considered
Alternatively, please consider making the plasma config part of the rollup config.

@tchardin
Copy link
Contributor

Hey @tuxcanfly, thanks for the suggestion. We do have a last PR to complete the op-plasma implementation that will add a proper plasma switch as part of system config. Once that is done, the flag will tell derivation whether to interpret the first byte of the tx data as a derivation version or commitment version at a given block. Will make sure to reflect that in the spec.

@tynes
Copy link
Contributor

tynes commented Mar 4, 2024

@tchardin Do you mean "at a given block" or do you mean "for a given block"? My understanding is there is no concept of going between plasma and rollup mode. "At a given block" implies you can switch between the two

@tuxcanfly Do you feel like this is sufficient?

@tchardin
Copy link
Contributor

tchardin commented Mar 4, 2024

After chat with @trianglesphere, we're going to go for bumping the batcher tx version byte to 1 and use that to detect whether tx data should be interpreted as plasma commitment and then keep a derivationVersion0 on the input batch to forward to the frame decoder. (Commitment type prefix is kept after the batcher tx type)

@tuxcanfly
Copy link
Contributor Author

@tchardin there is no batcher tx version right now, so this would be a new addition correct?

@tchardin
Copy link
Contributor

tchardin commented Mar 4, 2024

https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/derivation.md#batcher-transaction-format this is a bit ambiguous because it's also called derivationVersion in the implementation so in rollup mode the derivation version byte (0) is the batcher tx version byte too.

@tuxcanfly
Copy link
Contributor Author

@tchardin actually now that the rollup config includes usePlasma flag I'm not totally sure if this is required. We would need to move the version byte out of the frame txData into the plasma input. The additional byte also adds to the cost of L1 calldata that is submitted very frequently.

@tuxcanfly
Copy link
Contributor Author

@tchardin would this enable switching between rollup mode and plasma mode if we switch the derivation mode based on the version byte on a frame-by-frame basis?

@tuxcanfly
Copy link
Contributor Author

We can also enable challenges only for plasma mode by having a pre-condition that the version byte must be non-zero. This also derisks bugs or issues with the smart contract system itself, by having a fallback to the rollup mode.

@emilianobonassi
Copy link

@tchardin re: last PR to complete the op-plasma implementation, will get merged before ecotone?

@tchardin
Copy link
Contributor

tchardin commented Mar 8, 2024

@tuxcanfly still pending review but plan is using a tx data type byte as in https://github.com/latticexyz/redstone/pull/20/files, this means op plasma chain can still process regular input from L1 DA but not the way around, so a rollup mode chain would skip input commitments.

@emilianobonassi ecotone is already merged. If you mean before ecotone is activated, then yes hopefully will be merged before March 14.

@emilianobonassi
Copy link

@tchardin @tuxcanfly i think we can close since this is merged, is that correct?

ethereum-optimism/optimism#9845

@trianglesphere
Copy link
Contributor

Implemented in

ethereum-optimism/optimism#9845
#90

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

No branches or pull requests

5 participants