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

Tracking Issue: PeerDAS-KZG Integration (Consensus Layer) #42

Open
2 of 8 tasks
kevaundray opened this issue Jun 30, 2024 · 4 comments
Open
2 of 8 tasks

Tracking Issue: PeerDAS-KZG Integration (Consensus Layer) #42

kevaundray opened this issue Jun 30, 2024 · 4 comments
Labels
v1 needed for v1

Comments

@kevaundray
Copy link
Contributor

This will track the main integration PR(s) and also the refactor PRs which may need to be merged before integration.

Lighthouse

Reviewer: @jimmygchen

Integration

Refactor

Teku

Reviewer: @zilm13

Integration

Refactor

None yet

Lodestar

Reviewer: Possibly @matthewkeil / @g11tech

Integration

Refactor

None yet

Prysm

Prysm is integrating go-eth-kzg, its easier to track all clients in one place, so it is included here

Reviewer: @nalepae

Integration

Refactor

Nimbus

Reviewer: Still unclear; tagging relevant nim-kzg folks @tersec, @cheatfate, @jangko, @arnetheduck

Integration

None yet

Refactor

None yet

@kevaundray
Copy link
Contributor Author

I'd like to gather more information on nimbus integration as its the only library where the end-to-end integration is not clear.

From what I understand, it pulls in c-kzg using git-submodules into https://github.com/status-im/nim-kzg4844. It also seems to ignore the fact that c-kzg is a nimble package and "creates" a nimble package in nim-kzg4844 and just takes the relevant parts from c-kzg.

I do wonder if this can be simplified since the release cycle is (1) update c-kzg, then (2) update nim-kzg (3) update nimbus

@tersec
Copy link

tersec commented Jun 30, 2024

Nimbus has a few KZG-related PRs/branches:

some of which are more refactoring and some more integration, by the categorization employed in this issue.

Regarding:

From what I understand, it pulls in c-kzg using git-submodules into https://github.com/status-im/nim-kzg4844. It also seems to ignore the fact that c-kzg is a nimble package and "creates" a nimble package in nim-kzg4844 and just takes the relevant parts from c-kzg.

I do wonder if this can be simplified since the release cycle is (1) update c-kzg, then (2) update nim-kzg (3) update nimbus

nimbus-eth2 does not actively use Nimble at all. It does not care that any of it dependencies (nim-stew, nim-web3, nim-json-serialization, et cetera) are also Nimble packages. nim-kzg4844 is no different in this regard than anything else in https://github.com/status-im/nimbus-eth2/tree/unstable/vendor which uses a pinned commit via git submodule. This applies recursively: when nim-kzg4844 has dependencies such as c-kzg, it does this via git submodule too.

I'm not sure precisely what simplification you have in mind. We're working on improving the Nimble-based tooling so we can use that, but until then, yes, updating these recursively dependent modules involves multiple steps, sort of walking back up the dependency tree towards a (sub)root.

One of the integration challenges on our end thus far has been that all this lives in the das branch, not the main branch, so have an increasingly divergent PeerDAS codebase because of this increasingly divergent upstream dependency. Some of the changes are intrinsic, others are probably less so and might be able to live on main too. Regardless of what makes sense in terms of c-kzg branch management in isolation, it means that we're going to be looking at a somewhat messy merge because we (unless we switch nimbus-eth2 globally to use the das branch and not the main branch via nim-kzg4844's submodule commit) can't really gradually merge anything, and have to have this long-lived feature branch, which typically we avoid doing.

The ask here is, if people prioritize this that highly for Pectra, that there's movement on getting das merged into main at some point, well in advance of the first "full" integration expected as opposed to the PeerDAS devnets being this separate side-track which is thus far ignored by the main Pectra devnets.

Finally, in general, @agnxsh has been doing most of the PeerDAS-specific integration work on this, so he's a good person to communicate with as well on this topic in addition to some of the other people you've listed (e.g., @jangko and @cheatfate have been fixing the gcc 14 compatibility recently in c-kzg, but that's not especially PeerDAS-specific).

@kevaundray
Copy link
Contributor Author

The ask here is, if people prioritize this that highly for Pectra, that there's movement on getting das merged into main at some point, well in advance of the first "full" integration expected as opposed to the PeerDAS devnets being this separate side-track which is thus far ignored by the main Pectra devnets.

Noting that this is slated for this month, so somewhat soon -- I'm not sure when the full integration is expected to be

@kevaundray
Copy link
Contributor Author

I'll CC @asn-d6 and @jtraglia for visibiility

@kevaundray kevaundray added the v1 needed for v1 label Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1 needed for v1
Projects
None yet
Development

No branches or pull requests

2 participants