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

Process successful .signers-voting results #4360

Merged
merged 81 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
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 Feb 8, 2024
b5e765c
updated funcs name to reflect weight instead of slots
setzeus Feb 8, 2024
fa27b97
added vote func comments
setzeus Feb 8, 2024
b600a33
fix: accept votes for out of round
friedger Feb 1, 2024
4053af9
chore: better public keys, fix typos
friedger Feb 2, 2024
8f42eba
fix: use Point for aggregate public key
friedger Feb 6, 2024
8f7238b
chore: improve naming and comments
friedger Feb 8, 2024
74b40f5
feat: set aggregate public key once threshold is reached
obycode Feb 9, 2024
00bc983
chore: use uints for errors in signers-voting contract
obycode Feb 9, 2024
b2413c1
test: check for aggregate key approval event
obycode Feb 9, 2024
85b0ea2
fix: use weight in signer set (not number of slots)
obycode Feb 9, 2024
d98b6a7
fixed prepare-phase check & added cycle parameter to voting func
setzeus Feb 9, 2024
270bb36
updated broken tests
setzeus Feb 10, 2024
59bb4aa
feat: setup signers correctly when booting into Nakamoto
obycode Feb 11, 2024
970399f
new get-candidate-info getter
setzeus Feb 11, 2024
d8c8cfa
chore: fix style suggestions from PR review
obycode Feb 12, 2024
fe928d7
fix: fix issues in `get-candidate-info`
obycode Feb 12, 2024
afb43de
fix: properly retrieve signers in `advance_to_nakamoto`
obycode Feb 13, 2024
28f6333
fix: add test signers into peer config
obycode Feb 13, 2024
ccc20fc
refactor: improve testing functions related to signing
obycode Feb 13, 2024
2970970
chore: increase debug logging
obycode Feb 13, 2024
60f0e07
test: only vote in first nakamoto block of tenure
obycode Feb 13, 2024
407f3be
Add generate_aggregate_key to TestSigners
jferrant Feb 14, 2024
02bd070
test: use `generate_aggregate_key` between cycles
obycode Feb 14, 2024
00c5eba
fix: update `poly_commitments` in `generate_aggregate_key`
obycode Feb 14, 2024
1592b53
test: vote for aggregate key of next cycle in prepare phase
obycode Feb 14, 2024
c4be109
chore: update comment to match code
obycode Feb 14, 2024
eed2368
wip: try to merge `TestSigners` and `SelfSigner`
obycode Feb 14, 2024
699a1da
chore: update ed25519-dalek and rand libraries, use workspace versioning
kantai Feb 15, 2024
30c4c52
test: update signers key in tests
obycode Feb 16, 2024
d52b67e
fix: call `generate_aggregate_key` in `make_nakamoto_tenure`
obycode Feb 16, 2024
43acc8c
chore: remove `SelfSigner` which has been replaced with `TestSigners`
obycode Feb 16, 2024
69013f4
test: update `test_nakamoto_coordinator_10_tenures_and_extensions_10_…
obycode Feb 16, 2024
9cc1fd8
fix: Aaron's fix to the replay peer test failures
obycode Feb 16, 2024
d808c0b
Merge branch 'next' into feat/4354-aggregate-key
obycode Feb 16, 2024
4f7fa91
refactor: update uses of `TestSigners`
obycode Feb 16, 2024
8af5f9c
Merge branch 'next' into feat/4354-aggregate-key
obycode Feb 16, 2024
82df1f2
fix: fix TestSigners imports
obycode Feb 16, 2024
4e0180e
fix: allow duplicate keys in different voting round for the same cycle
obycode Feb 16, 2024
175d37e
fix: fix signers-voting tests
obycode Feb 16, 2024
e4c1e9c
chore: switch back to using weight instead of amount stacked
obycode Feb 16, 2024
a7c384c
fix: update for `slots` -> `weight` field naming change
obycode Feb 16, 2024
ffef7ee
fix: WIP fix the net tests after signer changes
obycode Feb 16, 2024
7c09d09
fix: TestPeer submits aggregate key votes automatically, inv tests **…
kantai Feb 17, 2024
52c7a59
test: alter NakamotoBootPlan to issue signer votes whenever possible …
kantai Feb 18, 2024
e220033
Merge remote-tracking branch 'origin/next' into feat/4354-aggregate-key
kantai Feb 18, 2024
746ea8b
test: fix merge of tests with other_peers
kantai Feb 18, 2024
6015cab
chore: fix warnings
obycode Feb 18, 2024
a8514f2
fix: attempt to add voting to the mockamoto tests
obycode Feb 18, 2024
2569934
use initial aggregate key boot contract for mockamoto, fix mockamoto …
kantai Feb 18, 2024
018ea78
fix: handle signer voting in nakamoto integration tests (WIP)
obycode Feb 19, 2024
024cd71
fix: update `correct_burn_outs` for new signer voting support
obycode Feb 19, 2024
8bbffeb
chore: remove hard-coded block height in `boot_to_epoch_3`
obycode Feb 19, 2024
ea4ecf9
Merge branch 'next' into feat/4354-aggregate-key
obycode Feb 19, 2024
6288b29
feat: add check for new round numbers
obycode Feb 19, 2024
192fdd5
first error message check
setzeus Feb 17, 2024
2230776
refactored helper make_signers_vote_for_aggregate_public_key & effect…
setzeus Feb 18, 2024
ff95043
removed unused err
setzeus Feb 18, 2024
ddad05b
catching all tenure-agnostic errs
setzeus Feb 18, 2024
d8cde7f
removed print statements & comment
setzeus Feb 18, 2024
ad1f014
formatted correctly
setzeus Feb 18, 2024
862e347
minor update
setzeus Feb 18, 2024
1ca5d86
test: separate out simple success test
obycode Feb 19, 2024
cd4a91d
test: add test for `ERR_INVALID_ROUND`
obycode Feb 19, 2024
955ac33
chore: update error codes to avoid overlap with .signers
obycode Feb 19, 2024
42a4efa
Merge branch 'next' into feat/4354-aggregate-key
obycode Feb 20, 2024
a92dbdb
refactor: delete `aggregate-public-keys` from pox-4
obycode Feb 20, 2024
f596c0b
fix: use `reward-cycle` parameter to get signer weight
obycode Feb 20, 2024
c8654f9
fix: update expected error after cycle fix in previous commit
obycode Feb 20, 2024
51f0c91
chore: assert that signers are not repeated in tests
obycode Feb 20, 2024
0fdaf62
refactor: refactoring suggestions from review
obycode Feb 20, 2024
252a98c
refactor: various refactoring suggestions from review
obycode Feb 21, 2024
e793b73
test: add test for out of window error
obycode Feb 20, 2024
70eb622
test: check for duplicate aggregate public key error
obycode Feb 20, 2024
3c0f858
test: test voting over multiple rounds
obycode Feb 20, 2024
911e5cd
test: try to vote early, before the prepare phase
obycode Feb 20, 2024
712c77e
test: verify that an old round can succeed after a new round has started
obycode Feb 21, 2024
ac80a88
feat: add round to `approved-aggregate-public-key` event
obycode Feb 21, 2024
91a1fd9
feat: add check for existing key in `signer_vote_if_needed`
obycode Feb 21, 2024
055fad3
Merge branch 'next' into feat/4354-aggregate-key
obycode Feb 21, 2024
d2e6ad8
Merge branch 'next' into feat/4354-aggregate-key
obycode Feb 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 4 additions & 18 deletions contrib/core-contract-tests/tests/pox-4/signers-voting.test.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,20 @@
import { Cl } from "@stacks/transactions";
import { beforeEach, describe, expect, it } from "vitest";
import { describe, expect, it } from "vitest";

const accounts = simnet.getAccounts();
const alice = accounts.get("wallet_1")!;
const bob = accounts.get("wallet_2")!;
const charlie = accounts.get("wallet_3")!;

const ERR_SIGNER_INDEX_MISMATCH = 10000;
const ERR_INVALID_SIGNER_INDEX = 10001;
const ERR_OUT_OF_VOTING_WINDOW = 10002
const ERR_OLD_ROUND = 10003;
const ERR_ILL_FORMED_AGGREGATE_PUBLIC_KEY = 10004;
const ERR_DUPLICATE_AGGREGATE_PUBLIC_KEY = 10005;
const ERR_DUPLICATE_VOTE = 10006;
const ERR_INVALID_BURN_BLOCK_HEIGHT = 10007

const KEY_1 = "123456789a123456789a123456789a123456789a123456789a123456789a010203";
const KEY_2 = "123456789a123456789a123456789a123456789a123456789a123456789ab0b1b2";
const SIGNERS_VOTING = "signers-voting";

describe("test signers-voting contract voting rounds", () => {
describe("test pox-info", () => {
it("should return correct burn-height", () => {
const { result:result1 } = simnet.callReadOnlyFn(SIGNERS_VOTING,
const { result: result1 } = simnet.callReadOnlyFn(SIGNERS_VOTING,
"reward-cycle-to-burn-height",
[Cl.uint(1)],
alice)
expect(result1).toEqual(Cl.uint(1050))

const { result:result2 } = simnet.callReadOnlyFn(SIGNERS_VOTING,
const { result: result2 } = simnet.callReadOnlyFn(SIGNERS_VOTING,
"reward-cycle-to-burn-height",
[Cl.uint(2)],
alice)
Expand All @@ -50,7 +36,7 @@ describe("test signers-voting contract voting rounds", () => {
})

it("should return true if in prepare phase", () => {
const { result:result999 } = simnet.callReadOnlyFn(SIGNERS_VOTING,
const { result: result999 } = simnet.callReadOnlyFn(SIGNERS_VOTING,
"is-in-prepare-phase",
[Cl.uint(999)],
alice)
Expand Down
3 changes: 3 additions & 0 deletions stackslib/src/burnchains/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,9 @@ impl PoxConstants {

// NOTE: first block in reward cycle is mod 1, so mod 0 is the last block in the
// prepare phase.
// TODO: I *think* the logic of `== 0` here requires some further digging.
// `mod 0` may not have any rewards, but it does not behave like "prepare phase" blocks:
// is it already a member of reward cycle "N" where N = block_height / reward_cycle_len
Comment on lines +598 to +600
Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

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.

reward_index == 0 || reward_index > u64::from(reward_cycle_length - prepare_length)
}
}
Expand Down
2 changes: 2 additions & 0 deletions stackslib/src/burnchains/tests/burnchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

use rand::rngs::ThreadRng;
use rand::thread_rng;
use rand_chacha::ChaChaRng;
use rand_core::SeedableRng;
use serde::Serialize;
use sha2::Sha512;
use stacks_common::address::AddressHashMode;
Expand Down
3 changes: 2 additions & 1 deletion stackslib/src/chainstate/burn/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ use std::fmt;
use std::io::Write;

use rand::seq::index::sample;
use rand::{Rng, SeedableRng};
use rand::Rng;
use rand_chacha::rand_core::SeedableRng;
use rand_chacha::ChaCha20Rng;
use ripemd::Ripemd160;
use rusqlite::{Connection, Transaction};
Expand Down
Loading