fix: dont log "couldn't connect to miner" in 2.5 #4768
Conversation
hstove
changed the title
| @@ -427,7 +430,7 @@ impl SignerRunLoop<Vec<OperationResult>, RunLoopCommand> for RunLoop { | |||
| &self.stacks_client, | |||
| event.as_ref(), | |||
| res.clone(), | |||
| current_reward_cycle, | |||
| ¤t_reward_cycle_info, | |||
There was a problem hiding this comment.
Don't think this needs to be by reference here as you already do as_ref() above.
| @@ -450,7 +453,7 @@ impl SignerRunLoop<Vec<OperationResult>, RunLoopCommand> for RunLoop { | |||
| } | |||
| } | |||
| // After processing event, run the next command for each signer | |||
| signer.process_next_command(&self.stacks_client, current_reward_cycle); | |||
| signer.process_next_command(&self.stacks_client, ¤t_reward_cycle_info); | |||
There was a problem hiding this comment.
similarly here. Don't think you need the reference.
| @@ -516,7 +522,7 @@ impl Signer { | |||
| // We cannot execute a DKG command if we are not the coordinator | |||
| Some(self.get_coordinator_dkg().0) | |||
| } else { | |||
| self.get_coordinator_sign(current_reward_cycle).0 | |||
| self.get_coordinator_sign(¤t_reward_cycle_info).0 | |||
There was a problem hiding this comment.
Simiarly don't htink you need the reference here.
| @@ -653,13 +659,18 @@ impl Signer { | |||
| let coordinator_pubkey = if Self::is_dkg_message(&packet.msg) { | |||
| self.get_coordinator_dkg().1 | |||
| } else { | |||
| self.get_coordinator_sign(current_reward_cycle).1 | |||
| self.get_coordinator_sign(¤t_reward_cycle_info).1 | |||
|
A bit unrelated to this PR, but it would be nice if you updated the internal call to get_node_epoch in process_dkg to go away (this was done in a sep branch that also added epoch to reward cycle info, but that branch is dead). Just need to pass the reward cycle info down to it. Its not a blocker for this PR, so if you don't want to do it here, will make a ticket and do on top of this. However, would you mind fixing the references I mentioned above? Recommend running clippy on stacks-signer every time you open a PR |
|
Closing this - no longer relevant |
When the signer looks up the current coordinator during an active reward cycle, it assumes the miner should be the coordinator. In 2.5, this isn't the case. When DKG doesn't complete before the active reward cycle, the signer continues to lookup the coordinator, but it still outputs this error log.
TLDR: the signer's coordinator logic is fine, as it already falls back to the DKG coordinator, but it shouldn't log this error in 2.5.
To implement this, I've extended the
RewardCycleInfostruct to include the current epoch, and I pass around this struct instead of justcurrent_reward_cyclein a few places.