-
Notifications
You must be signed in to change notification settings - Fork 678
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
Process successful .signers-voting results #4360
Merged
Merged
Changes from all commits
Commits
Show all changes
81 commits
Select commit
Hold shift + click to select a range
6b87b77
updated error message format to match pox-4 messages
setzeus b5e765c
updated funcs name to reflect weight instead of slots
setzeus fa27b97
added vote func comments
setzeus b600a33
fix: accept votes for out of round
friedger 4053af9
chore: better public keys, fix typos
friedger 8f42eba
fix: use Point for aggregate public key
friedger 8f7238b
chore: improve naming and comments
friedger 74b40f5
feat: set aggregate public key once threshold is reached
obycode 00bc983
chore: use uints for errors in signers-voting contract
obycode b2413c1
test: check for aggregate key approval event
obycode 85b0ea2
fix: use weight in signer set (not number of slots)
obycode d98b6a7
fixed prepare-phase check & added cycle parameter to voting func
setzeus 270bb36
updated broken tests
setzeus 59bb4aa
feat: setup signers correctly when booting into Nakamoto
obycode 970399f
new get-candidate-info getter
setzeus d8c8cfa
chore: fix style suggestions from PR review
obycode fe928d7
fix: fix issues in `get-candidate-info`
obycode afb43de
fix: properly retrieve signers in `advance_to_nakamoto`
obycode 28f6333
fix: add test signers into peer config
obycode ccc20fc
refactor: improve testing functions related to signing
obycode 2970970
chore: increase debug logging
obycode 60f0e07
test: only vote in first nakamoto block of tenure
obycode 407f3be
Add generate_aggregate_key to TestSigners
jferrant 02bd070
test: use `generate_aggregate_key` between cycles
obycode 00c5eba
fix: update `poly_commitments` in `generate_aggregate_key`
obycode 1592b53
test: vote for aggregate key of next cycle in prepare phase
obycode c4be109
chore: update comment to match code
obycode eed2368
wip: try to merge `TestSigners` and `SelfSigner`
obycode 699a1da
chore: update ed25519-dalek and rand libraries, use workspace versioning
kantai 30c4c52
test: update signers key in tests
obycode d52b67e
fix: call `generate_aggregate_key` in `make_nakamoto_tenure`
obycode 43acc8c
chore: remove `SelfSigner` which has been replaced with `TestSigners`
obycode 69013f4
test: update `test_nakamoto_coordinator_10_tenures_and_extensions_10_…
obycode 9cc1fd8
fix: Aaron's fix to the replay peer test failures
obycode d808c0b
Merge branch 'next' into feat/4354-aggregate-key
obycode 4f7fa91
refactor: update uses of `TestSigners`
obycode 8af5f9c
Merge branch 'next' into feat/4354-aggregate-key
obycode 82df1f2
fix: fix TestSigners imports
obycode 4e0180e
fix: allow duplicate keys in different voting round for the same cycle
obycode 175d37e
fix: fix signers-voting tests
obycode e4c1e9c
chore: switch back to using weight instead of amount stacked
obycode a7c384c
fix: update for `slots` -> `weight` field naming change
obycode ffef7ee
fix: WIP fix the net tests after signer changes
obycode 7c09d09
fix: TestPeer submits aggregate key votes automatically, inv tests **…
kantai 52c7a59
test: alter NakamotoBootPlan to issue signer votes whenever possible …
kantai e220033
Merge remote-tracking branch 'origin/next' into feat/4354-aggregate-key
kantai 746ea8b
test: fix merge of tests with other_peers
kantai 6015cab
chore: fix warnings
obycode a8514f2
fix: attempt to add voting to the mockamoto tests
obycode 2569934
use initial aggregate key boot contract for mockamoto, fix mockamoto …
kantai 018ea78
fix: handle signer voting in nakamoto integration tests (WIP)
obycode 024cd71
fix: update `correct_burn_outs` for new signer voting support
obycode 8bbffeb
chore: remove hard-coded block height in `boot_to_epoch_3`
obycode ea4ecf9
Merge branch 'next' into feat/4354-aggregate-key
obycode 6288b29
feat: add check for new round numbers
obycode 192fdd5
first error message check
setzeus 2230776
refactored helper make_signers_vote_for_aggregate_public_key & effect…
setzeus ff95043
removed unused err
setzeus ddad05b
catching all tenure-agnostic errs
setzeus d8cde7f
removed print statements & comment
setzeus ad1f014
formatted correctly
setzeus 862e347
minor update
setzeus 1ca5d86
test: separate out simple success test
obycode cd4a91d
test: add test for `ERR_INVALID_ROUND`
obycode 955ac33
chore: update error codes to avoid overlap with .signers
obycode 42a4efa
Merge branch 'next' into feat/4354-aggregate-key
obycode a92dbdb
refactor: delete `aggregate-public-keys` from pox-4
obycode f596c0b
fix: use `reward-cycle` parameter to get signer weight
obycode c8654f9
fix: update expected error after cycle fix in previous commit
obycode 51f0c91
chore: assert that signers are not repeated in tests
obycode 0fdaf62
refactor: refactoring suggestions from review
obycode 252a98c
refactor: various refactoring suggestions from review
obycode e793b73
test: add test for out of window error
obycode 70eb622
test: check for duplicate aggregate public key error
obycode 3c0f858
test: test voting over multiple rounds
obycode 911e5cd
test: try to vote early, before the prepare phase
obycode 712c77e
test: verify that an old round can succeed after a new round has started
obycode ac80a88
feat: add round to `approved-aggregate-public-key` event
obycode 91a1fd9
feat: add check for existing key in `signer_vote_if_needed`
obycode 055fad3
Merge branch 'next' into feat/4354-aggregate-key
obycode d2e6ad8
Merge branch 'next' into feat/4354-aggregate-key
obycode File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
Oops, something went wrong.
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.
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 added this comment here for some visibility -- this may impact block inventory logic, right, @jcnelson?
The "0" reward index isn't really a prepare phase block for cycle N: it's a member of reward cycle N, not N-1.
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.
Changing this would impact far more than block inventory logic. I'm afraid this is consensus-level logic.
Fundamentally, there's an unsoundness in the code for deciding whether or not a block height is in the prepare phase or reward phase. In some places, like this one and others, a block whose height modulo the reward cycle length is 0 is in the prepare phase, since the reward phase begins when the block height modulo the reward cycle length is 1 or greater. In other places, like
static_block_height_to_reward_cycle
, a block is in the reward phase if its height modulo the reward cycle length is 0.Unfortunately, this code here (as well as the code paths that declare the start of a reward cycle to be at each block whose heights modulo the reward cycle length is 1) is used all over the place in the blockchain. We can't change it.
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.
Ah, yes, I see. That's fine -- I think that nakamoto code just needs to be careful to use
reward_cycle_of_prepare_phase()
in order to get the reward cycle that corresponds to a prepare phase block. Otherwise, that block is very susceptible to off-by-one errors.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 also different from
is-in-prepare-phase
in the voting contract.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.
Also
reward-cycle-to-burn-height
in the voting contract is inconsistent with the code here.