-
Notifications
You must be signed in to change notification settings - Fork 156
DA stake table endpoint #3700
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
DA stake table endpoint #3700
Conversation
ss-es
left a comment
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 think the da stake table by epoch endpoint should be simplified (just naively query the membership coordinator, without a cur_epoch guard or catchup). looks good to me otherwise
sequencer/src/api.rs
Outdated
| let highest_epoch = self | ||
| .consensus() | ||
| .await | ||
| .read() | ||
| .await | ||
| .cur_epoch() | ||
| .await | ||
| .map(|e| e + 1); | ||
| if epoch > highest_epoch { | ||
| return Err(anyhow::anyhow!( | ||
| "requested DA stake table for epoch {epoch:?} is beyond the current epoch + 1 \ | ||
| {highest_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.
for the DA stake table by epoch, I think this is not necessary. we can just query for the future DA stake table (and actually, it would be useful to not block requests far in the future, since we can use this to see that the upgrade took effect and the DA change was correctly applied)
sequencer/src/api.rs
Outdated
| let mem = self | ||
| .consensus() | ||
| .await | ||
| .read() | ||
| .await | ||
| .membership_coordinator | ||
| .stake_table_for_epoch(epoch) | ||
| .await?; |
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 is not necessary, we don't need to trigger catchup and should just query the membership coordinator directly
0e71788 to
30613ee
Compare
30613ee to
95ae5d3
Compare
ss-es
left a comment
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.
looks good to me
* DA stake table endpoint * fixes * remove redundant attribute
Adds an endpoint for da-stake-table/:epoch and da-stake-table/current