Do not accept signatures, block proposals, or block responses for blocks from different reward cycles

[jferrant]

Expands upon #5612

Closes #5611

[hstove]

LGTM!

[kantai]

LGTM -- once the integration test is added with injected stackerdb messages, I'll approve.

[jferrant]

LGTM -- once the integration test is added with injected stackerdb messages, I'll approve.

I don't know what the purpose of injecting stackerdb messages is. I do test already that signers ignore messages based on reward cycle in both integration tests. If we want to have further checks on validity of messages we will need the following implemented #5671.

I think this PR should be merged regardless.

EDIT: Actually, I think there is one additional path that I do not test that I could check with an injection as you suggest. Let me give that a shot quickly.

[hstove]

I don't know what the purpose of injecting stackerdb messages is.

Maybe you're thinking the same thing (you later said there was a scenario you could think of), but to me the purpose is that signers properly ignore/handle block responses that come from these "other" signers. For example, it could be a malicious signer, or it could just be an un-upgraded signer.

The tests you have assert that signers ignore block proposals, but really what triggered this issue was not ignoring block responses. I do think the code you have in this PR covers the functionality we want already, I just also think this test scenario would be helpful.

[jferrant]

I don't know what the purpose of injecting stackerdb messages is.

Maybe you're thinking the same thing (you later said there was a scenario you could think of), but to me the purpose is that signers properly ignore/handle block responses that come from these "other" signers. For example, it could be a malicious signer, or it could just be an un-upgraded signer.

The tests you have assert that signers ignore block proposals, but really what triggered this issue was not ignoring block responses. I do think the code you have in this PR covers the functionality we want already, I just also think this test scenario would be helpful.

It kind of already does this. It handles this case by keeping incoming and outgoing signers around and forcing the valid signers to respond. I verify that none of the "not your cycle signers" do nothing in response to the block proposals as well as the signer signatures. I guess I don't explicitly write to the "wrong" signers signer db, but the slot it is written to does not matter in the signer. They don't care who the originator of a message is so I am still testing the same signer side code. I will add one to accomodate the threshold test though to aid in debugging if we ever have some other bug occur specifically at that point.

[aldur]

Release playbook for the code in this PR:

1. Except for flaky test and clippy tests, CI of this PR must pass;
2. Once merged to develop, cut a release branch from there;
3. The release branch is opened as a PR against master (CC @wileyj);
4. Block replay (takes roughly 12h);
5. Run a signer and a miner on mainnet for a limited time with the updated code (in parallel with 4);
6. Deploy to testnet (in parallel with 4 and 5);
7. Release (assuming all previous steps are successful);

Targeting Monday, but knowing most folks are traveling this might be released on Tuesday.

[hstove]

LGTM, nice test(s)!