Skip to content

refactor: submission libs#12063

Merged
LHerskind merged 5 commits intomasterfrom
lh/12062-submission-libs
Feb 26, 2025
Merged

refactor: submission libs#12063
LHerskind merged 5 commits intomasterfrom
lh/12062-submission-libs

Conversation

@LHerskind
Copy link
Contributor

@LHerskind LHerskind commented Feb 18, 2025

Fixes #12062, to take us part of the way for compartmentalisation. Changes a bunch of libraries to have block and proof submission be in libraries similar to others. Next steps are to split the storage access more, such that a case of ignition with empty blocks can be run while skipping a bunch of the fee computations etc. For that part, we also gotta consider how we should structure the storage for simpler reuse, so it is to be done separate.

Copy link
Contributor Author

LHerskind commented Feb 18, 2025

@LHerskind LHerskind changed the title chore: coarse cleanup refactor: submission libs Feb 18, 2025
@LHerskind LHerskind force-pushed the lh/12062-submission-libs branch from 2a91933 to 002a951 Compare February 18, 2025 17:35
@LHerskind LHerskind force-pushed the lh/12062-submission-libs branch from 002a951 to 0bbe617 Compare February 24, 2025 17:20
@LHerskind LHerskind force-pushed the lh/12062-submission-libs branch from 0bbe617 to 53c3c67 Compare February 25, 2025 08:25
@LHerskind LHerskind force-pushed the lh/12062-submission-libs branch from 53c3c67 to f81364f Compare February 25, 2025 09:40
@LHerskind LHerskind linked an issue Feb 25, 2025 that may be closed by this pull request
@LHerskind LHerskind force-pushed the lh/12062-submission-libs branch 2 times, most recently from ae110bb to c4f6397 Compare February 25, 2025 16:34
IVerifier epochProofVerifier;
IInbox inbox;
IOutbox outbox;
uint256 version;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unrelated to this PR, but what is this version supposed to represent? Is this supposed to function like our chain ID?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently just to make sure that we can "scope" messages to a specific rollup instance. So the thing gets a bit weird here with chain ids and such.

Normally, you would have that the upgrade keeps the same chainid, and if there is a fork, the non-canonical gotta get a new one. In our case, if the instances are setup to be immutable, it cannot really just update its. So it is there as a version identifier.

So within the same "family", e.g., instances that are added to the same registry, we should also have different versions such that a transaction intended for 4 don't get played on 3. (there are a few updates to do to the registry if we really want to check these things and make the lookup better).

The reason that it is not a huge issue if some other instance deployed "outside" of the family is using the same version mainly comes from how our transactions are proven. As a transaction is referring to some header in the past to prove against, if state varies it won't be there. And if state is the same, is should be on the registry, or it should generally be possible to see it as a "worthless" variant.

On the chainid, it is a more painful thing really, and I don't know the best way forward for us there. Maybe it is just storing it in the registry, and then let the canonical use that 🤷 But users would expect it to move along the newest and to denote the "aztec network"

Comment on lines +118 to +119
rollupStore.blocks[blockNumber] =
toBlockLog(_args, blockNumber, components.congestionCost, components.provingCost);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a little surprised this doesn't occur within the STFLib? In your mind, what lib is responsible for what?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there will be another round of cleaning on top of this later, it is mainly getting the coarse split in place such that it is some of the way, the 80/20.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, ye, remembered, this was the only place using it, and the only reason that function even existed was to avoid a stack too deep.

// We are using this library such that we can more easily "link" just a larger external library
// instead of a few smaller ones.

library ExtRollupLib {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Really nice to have submitEpochRootProof and propose in here.

@LHerskind LHerskind force-pushed the lh/12062-submission-libs branch from c4f6397 to 30036b3 Compare February 26, 2025 10:12
Base automatically changed from lh/11941 to master February 26, 2025 10:39
@LHerskind LHerskind force-pushed the lh/12062-submission-libs branch from 30036b3 to fc82e91 Compare February 26, 2025 10:40
@LHerskind LHerskind marked this pull request as ready for review February 26, 2025 10:40
@LHerskind
Copy link
Contributor Author

LHerskind commented Feb 26, 2025

Needs to update the cheat code used by the anvil watcher here to correctly update the proven storage slot.

@LHerskind LHerskind force-pushed the lh/12062-submission-libs branch from fc82e91 to 9a4b23c Compare February 26, 2025 13:41
@LHerskind LHerskind force-pushed the lh/12062-submission-libs branch from 9a4b23c to 2084d12 Compare February 26, 2025 14:42
@LHerskind LHerskind merged commit 92204e9 into master Feb 26, 2025
10 checks passed
@LHerskind LHerskind deleted the lh/12062-submission-libs branch February 26, 2025 15:20
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.

feat: block and proof submission libs

2 participants