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

feat: gather v0 block signatures from stackerdb #4807

Merged
merged 11 commits into from
May 31, 2024
Merged
Prev Previous commit
Next Next commit
feat: remove aggregate key related stuff from sign coordinator
  • Loading branch information
hstove committed May 23, 2024
commit 2275aa877d7524b83929d63ea3f894d37a1e4adb
68 changes: 16 additions & 52 deletions testnet/stacks-node/src/nakamoto_node/miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,19 +280,6 @@ impl BlockMinerThread {
})
})?;

let reward_cycle = self
.burnchain
.pox_constants
.block_height_to_reward_cycle(
self.burnchain.first_block_height,
self.burn_block.block_height,
)
.ok_or_else(|| {
NakamotoNodeError::SigningCoordinatorFailure(
"Building on a burn block that is before the first burn block".into(),
)
})?;

let reward_info = match sort_db.get_preprocessed_reward_set_of(&tip.sortition_id) {
Ok(Some(x)) => x,
Ok(None) => {
Expand Down Expand Up @@ -328,19 +315,14 @@ impl BlockMinerThread {
};

let miner_privkey_as_scalar = Scalar::from(miner_privkey.as_slice().clone());
let mut coordinator = SignCoordinator::new(
&reward_set,
reward_cycle,
miner_privkey_as_scalar,
Some(aggregate_public_key),
&stackerdbs,
&self.config,
)
.map_err(|e| {
NakamotoNodeError::SigningCoordinatorFailure(format!(
"Failed to initialize the signing coordinator. Cannot mine! {e:?}"
))
})?;
let mut coordinator =
SignCoordinator::new(&reward_set, miner_privkey_as_scalar, &self.config).map_err(
|e| {
NakamotoNodeError::SigningCoordinatorFailure(format!(
"Failed to initialize the signing coordinator. Cannot mine! {e:?}"
))
},
)?;

*attempts += 1;
let signature = coordinator.begin_sign_v1(
Expand Down Expand Up @@ -417,33 +399,15 @@ impl BlockMinerThread {
));
};

let reward_cycle = self
.burnchain
.pox_constants
.block_height_to_reward_cycle(
self.burnchain.first_block_height,
self.burn_block.block_height,
)
.ok_or_else(|| {
NakamotoNodeError::SigningCoordinatorFailure(
"Building on a burn block that is before the first burn block".into(),
)
})?;

let miner_privkey_as_scalar = Scalar::from(miner_privkey.as_slice().clone());
let mut coordinator = SignCoordinator::new(
&reward_set,
reward_cycle,
miner_privkey_as_scalar,
None,
&stackerdbs,
&self.config,
)
.map_err(|e| {
NakamotoNodeError::SigningCoordinatorFailure(format!(
"Failed to initialize the signing coordinator. Cannot mine! {e:?}"
))
})?;
let mut coordinator =
SignCoordinator::new(&reward_set, miner_privkey_as_scalar, &self.config).map_err(
|e| {
NakamotoNodeError::SigningCoordinatorFailure(format!(
"Failed to initialize the signing coordinator. Cannot mine! {e:?}"
))
},
)?;

*attempts += 1;
let signature = coordinator.begin_sign_v0(
Expand Down
26 changes: 2 additions & 24 deletions testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,7 @@ impl SignCoordinator {
/// * `aggregate_public_key` - the active aggregate key for this cycle
pub fn new(
reward_set: &RewardSet,
reward_cycle: u64,
message_key: Scalar,
aggregate_public_key: Option<Point>,
stackerdb_conn: &StackerDBs,
config: &Config,
// v1: bool,
) -> Result<Self, ChainstateError> {
Expand Down Expand Up @@ -281,7 +278,7 @@ impl SignCoordinator {
})
.collect::<Result<HashMap<_, _>, ChainstateError>>()?;

let mut coordinator: FireCoordinator<Aggregator> = FireCoordinator::new(coord_config);
let coordinator: FireCoordinator<Aggregator> = FireCoordinator::new(coord_config);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be moved into the #[cfg(test)] block? Also, can the imports for wsts be moved there as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both the cfg(test) block and the "non test" block return a SignCoordinator, which has a coordinator field. So, we'd have to return some other type if we wanted to only include the wsts stuff in the cfg(test) block. Ultimately, this whole file relies on both the v0 and v1 stuff (ie begin_sign_v0 and begin_sign_v1) - there's no easy fix other than separate files / structs IMO

#[cfg(test)]
{
// In test mode, short-circuit spinning up the SignCoordinator if the TEST_SIGNING
Expand All @@ -294,7 +291,7 @@ impl SignCoordinator {
if replaced_other {
warn!("Replaced the miner/coordinator receiver of a prior thread. Prior thread may have crashed.");
}
let mut sign_coordinator = Self {
let sign_coordinator = Self {
coordinator,
message_key,
receiver: Some(receiver),
Expand All @@ -306,28 +303,9 @@ impl SignCoordinator {
signer_entries: signer_public_keys,
weight_threshold: threshold,
};
if let Some(aggregate_public_key) = aggregate_public_key {
sign_coordinator
.coordinator
.set_aggregate_public_key(Some(aggregate_public_key));
}
return Ok(sign_coordinator);
}
}
if let Some(aggregate_public_key) = aggregate_public_key {
let party_polynomials = get_signer_commitments(
is_mainnet,
reward_set_signers.as_slice(),
stackerdb_conn,
reward_cycle,
&aggregate_public_key,
)?;
if let Err(e) = coordinator
.set_key_and_party_polynomials(aggregate_public_key.clone(), party_polynomials)
{
warn!("Failed to set a valid set of party polynomials"; "error" => %e);
};
}

let (receiver, replaced_other) = STACKER_DB_CHANNEL.register_miner_coordinator();
if replaced_other {
Expand Down