-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Feat: multiframe altda channel #12400
base: develop
Are you sure you want to change the base?
Feat: multiframe altda channel #12400
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #12400 +/- ##
===========================================
- Coverage 64.32% 64.14% -0.19%
===========================================
Files 52 52
Lines 4348 4348
===========================================
- Hits 2797 2789 -8
- Misses 1376 1385 +9
+ Partials 175 174 -1
Flags with carried forward coverage won't be shown. Click here to find out more. |
This looks like generally a good change to me, we will get around to review more thoroughly after we complete holocene code freeze next week |
@tynes any updates here? |
878a216
to
1a84f49
Compare
+1 for interest in this update on Polymer's side as well. |
1a84f49
to
bb0ba93
Compare
ping, gentle push to see if it would be possible to take another look at this guy |
This is important for Batched Commitments as well :) @samlaf do you think it would be good to have a new |
* ci: add test-golang workflow * ci: delete circleci yml file This was used by op, we are moving to using github actions for our fork * ci: fix linter by using golangci-lint action * ci: use golangci-lint v1.61 to match mise.toml requirement * ci: use mise in ci to install correct versions of dependencies * ci: remove setup-go step because go already installed by mise in previous step * ci: build contracts that are needed for op-e2e tests * ci: use caching for forge artifacts * ci: remove slow op-program build don't think we are using it... testing * ci: make go-lint job run in parallel, and update version to try to fix errors * ci: add go modules caching to speed up workflow * ci: add explicit go mod download
Probably yes, haven't fully thought through your proposal. Will answer in your proposal and we can continue discussion there. |
bb0ba93
to
e0a8073
Compare
Description
Currently altda forces using "calldata" channels, which restrict their txData output to a single frame at a time. EigenDA (and probably other altdas) support larger blobs than the default 128KB frame size. This PR allows channels to have the altDA da type, which allows them to output a much larger number of frames when queried for txData. These frames are then concatenated and sent as a large blob to the altda source.
Tests
No new tests because there isn't really any change in behaviour here since this is a small repurpose of frame concatenation logic that was already there (curious to see if the ci here will pass). The derivation pipeline also doesn't need any changes because it already can read multiple concatenated frames. Perhaps an e2e test to show that an op-node is able to derive the concatenated frames would be useful though?
Additional context
An alternative here would have been to still read single frame at a time, but increase the MaxL1FrameSize variable. This approach requires more changes, and the derivation pipeline also has hardcoded limits which would have needed to change. Also keeping small frames allows fallbacking to ethDA automatically if ever needed, potentially even inside a single channel.
Metadata