Update ValidateRejectCode from InvalidBlock to UnknownParent where appropriate and add additional reject codes

[jferrant]

No description provided.

[obycode]

This is good. Wish I had thought of this case when I originally made this change ☹️. Should we combine this with the discussed change to resubmit the proposal N times if it fails for certain reasons (like invalid parent block)?

[jferrant]

This is good. Wish I had thought of this case when I originally made this change ☹️. Should we combine this with the discussed change to resubmit the proposal N times if it fails for certain reasons (like invalid parent block)?

I don't think we actually need to. I misread the code. Because you set prior_evaluation to None when it thinks it should_reevaluate_block, it won't hit the codepath that early exits. So its all good! You had it right, just missed this one spot! (EDIT: which also makes sense why your test worked. It was testing a same block being reproposed but it just didn't check for the specific InvalidParentBlock case)

[obycode]

Actually, I think this still does not catch the case we're seeing. The block validation endpoint is responding with InvalidBlock in this case. I think it would require some changes in that endpoint to differentiate the different errors better.

[jferrant]

Actually, I think this still does not catch the case we're seeing. The block validation endpoint is responding with InvalidBlock in this case. I think it would require some changes in that endpoint to differentiate the different errors better.

Great catch. This makes me think actually that maybe InvalidParentBlock should NOT be reevaluated. This happens when a miner is attempting to reorg actually. Not sure that should ever be reconsidered. definitely conflated this error code with the validateReject reason code which is different XD

[obycode]

Thanks for the update. Yes, I think this looks good, but the minor challenge here is that this requires signers to update their nodes at the same time as updating their signers. Updating the node and not the signer will result in deserialization errors.

[obycode]

👍

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.79%. Comparing base (9a19b37) to head (aaabb90).
Report is 49 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #6027      +/-   ##
===========================================
+ Coverage    79.64%   83.79%   +4.15%     
===========================================
  Files          528      528              
  Lines       385780   386301     +521     
  Branches       323      323              
===========================================
+ Hits        307247   323702   +16455     
+ Misses       78525    62591   -15934     
  Partials         8        8              

Files with missing lines	Coverage Δ
stackslib/src/net/api/postblock_proposal.rs	79.31% <100.00%> (-5.71%)	⬇️
stackslib/src/net/api/tests/postblock_proposal.rs	41.11% <100.00%> (-47.06%)	⬇️
...net/stacks-node/src/tests/nakamoto_integrations.rs	87.40% <100.00%> (+0.03%)	⬆️
testnet/stacks-node/src/tests/signer/v0.rs	58.75% <ø> (-1.64%)	⬇️

... and 195 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e505550...aaabb90. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.