-
Notifications
You must be signed in to change notification settings - Fork 87
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
update comment to match implementation #2282
Merged
alysiahuggins
merged 6 commits into
main
from
2278-cp-audit-update-comment-to-match-logic-on-lightclientsol
Dec 6, 2024
Merged
update comment to match implementation #2282
alysiahuggins
merged 6 commits into
main
from
2278-cp-audit-update-comment-to-match-logic-on-lightclientsol
Dec 6, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alysiahuggins
requested review from
philippecamacho,
ImJeremyHe,
sveitser and
jbearer
as code owners
November 14, 2024 04:27
…n the state history
…on-lightclientsol
zacshowa
approved these changes
Dec 6, 2024
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!
alysiahuggins
deleted the
2278-cp-audit-update-comment-to-match-logic-on-lightclientsol
branch
December 6, 2024 15:17
tbro
pushed a commit
that referenced
this pull request
Dec 6, 2024
* align comment with implementation * added more clarity * comment format * clearer comment * update comment to make it clear that there may be outdated elements in the state history
tbro
pushed a commit
that referenced
this pull request
Dec 6, 2024
* align comment with implementation * added more clarity * comment format * clearer comment * update comment to make it clear that there may be outdated elements in the state history
sveitser
added a commit
that referenced
this pull request
Dec 10, 2024
* Initial draft for minimal L1 stake table * Add Schnorr verifying key * fix solhint error: named mapping keys and values * WIP: contract stake table types to rust types Missing conversion from G2 Affine to BLSPubKey. * rename: SimpleStakeTable -> PermissionedStakeTable * PermissionedStakeTable contract: add DA flag - Update bindings. - Add tests for event emission. * Allow setting an initial stake table on deployment - The stake table isn't useful without stakers I think it makes sense to require it to be provided on deployment. * deploy: load initial stake table from toml file * Use serde instead of unsafe rust We will add some more ergonomic code for conversion to jellyfish at some point. This code can be used as a stop gap until then. * Update utils/src/deployer.rs Co-authored-by: Alysia Tech <[email protected]> * WIP: load initial stake table from file * Move solidity <-> jf code to contract adapter * Load initial stake table from file * remove unused imports * remove unused module * dev-node: fix arguments to deploy function * fix clippy * fix Cargo.toml for bindings generation breaks if we use: `serde.workspace = true` * combine adding and removing into single event * update comment to match implementation (#2282) * align comment with implementation * added more clarity * comment format * clearer comment * update comment to make it clear that there may be outdated elements in the state history * update query-service * db max connections = 25 for dev node tests * update query-service * Fix no-storage decides * Branches -> tags * 2368 stake table registration with fixed stake (#2365) * add stake table tests * remove stake types * verify token allowance, balance and reprioritize verification order on registration * set the fixed stake amount, added related tests, updated data types * add more verification checks to the withdraw function * updated errror types * added TODO statements in comments to be explicit about outdated functions that need to be updated to the new spec * fix env var for permissioned_stake_table * Add test for jf / contract types conversions * cleanup: pass rng * load stake table from toml file (#2377) * change lc proxy addr * fix deploy-sequencer-contracts in docker compose * test for stake table from toml file * initial stake table toml file * exclude toml file in typos * remove custom deser * remove unnecessary rand dependency --------- Co-authored-by: Alysia Tech <[email protected]> Co-authored-by: tbro <[email protected]> Co-authored-by: imabdulbasit <[email protected]> Co-authored-by: Artemii Gerasimovich <[email protected]> Co-authored-by: Abdul Basit <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #2278
This PR:
Updates comment to match implementation
This PR does not:
Doesn't change any functionality
Key places to review:
The comment