Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Deterministic L1 starting block selection (Config in advance) #342
base: celo-rebase-12
Are you sure you want to change the base?
Deterministic L1 starting block selection (Config in advance) #342
Changes from 15 commits
60f36c4
e0ab6c3
8d6a0d4
12b9558
b7ebbab
b4c7863
4a08a3c
7d4569d
dde8dce
9264244
c047c78
b311c43
cfba705
c2daa1d
702e087
3591c74
a9897b3
d56f9ed
402c2f1
ec03587
c027dab
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you renamed the arg, but the comment still references the old name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and following steps are dependent on that the point-in-time has actually passed (in wall-time and on-chain in terms of prior epoch justifications).
In the L1StartBlock determining code you wait for that time to pass prior to calling this method.
I think it would make sense to include the waiting logic here instead, since the function basically doesn't do anything when called before that time anyways.
Additionally, you could make the point in time arg optional, and if it is
nil
it takestime.Now()
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to wait in this method, makes sense thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have preferred to use the BeaconAPI for this, but it seems like you would have to process the BLS signatures and the graph of
source
totarget
in the attestations yourself for each block in the epoch...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think I can use the
/eth/v1/beacon/states/{state_id}/finality_checkpoints
endpoint for this, I wasn't able to test before since I hadn't found an rpc that supported historical requests for/eth/v1/beacon/states/{state_id}/finality_checkpoints
but https://ethereum-holesky-beacon-api.publicnode.com seems to. So I'm planning to remove the reliance on beaconcha.inThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there some lib for that kind of stuff?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found these two options:
But they both just provide clients for the beacon-rpc, so I'd still need to write a client for the beaconcha.in requests, it didn't seem worth it to import all that stuff for one method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use something like
go-swagger
to generate the schema (if you didn't use that already) directly fro m the swagger file, but I don't think it's worth itThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds interesting, I will have a look, where is the swagger file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://ethereum.github.io/beacon-APIs/releases/v3.0.0/beacon-node-oapi.json
The beacon-api specs is basically just a render of that