fix: look up quorum by actual `quorum_index` not by array position by xdustinface · Pull Request #406 · dashpay/rust-dashcore

xdustinface · 2026-02-02T22:37:23Z

We currently look up the quorums by their position in the vector which leads to invalid quroums being selected for IS verification. This PR changes it to look up by actual quorum index.

Summary by CodeRabbit

Bug Fixes
- Improved quorum lookup reliability by replacing crash-prone index retrieval with field-based lookup, now returning informative errors when quorums are unavailable instead of panicking.
Tests
- Added comprehensive test coverage for quorum retrieval, including edge cases and error scenarios to ensure stable behavior.

coderabbitai · 2026-02-02T22:37:40Z

📝 Walkthrough

Walkthrough

The changes improve quorum lookup reliability by replacing direct index-based array access with field-based lookup, and introducing proper error handling for missing quorum indices. The is_lock_quorum function now resolves quorums by their quorum_index field instead of array position, surfacing a new error variant when a quorum is not found. Comprehensive tests verify lookup correctness across various quorum orderings and edge cases.

Changes

Cohort / File(s)	Summary
Quorum Lookup Enhancement `dash/src/sml/masternode_list_engine/message_request_verification.rs`, `dash/src/sml/message_verification_error.rs`	Modified quorum retrieval from index-based array access to field-based lookup using `iter().find()`, eliminating panic on missing quorums. Added new `QuorumIndexNotFound(u16, CycleHash)` error variant and tests verifying lookup behavior across permutations, including scenarios with reordered and missing quorums.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 No more panic when quorums are lost,
We search by their field, not by cost,
Through rotated arrays we safely find,
Each quorum's true place, one step at a time! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title clearly and concisely summarizes the main change: fixing quorum lookup to use the actual quorum_index field instead of array position, which directly matches the primary objective of the PR.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/quorum-index-lookup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

ZocoLini · 2026-02-03T03:01:01Z

dash/src/sml/masternode_list_engine/message_request_verification.rs

+
+        let lock_data = hex::decode("01018d53e7997ead57409750942af0d5e0aafc06f852a9a52308f4781b6a8220298f00000000c6f9d8c63dd15937ea70aaddb7890daad42c91bf6818e2bf76d183d6f2d9215b4b5f84978fad9dde7ab52bdcc0674be891e9029cc1ef0cb01200000000000000a27c98836c4c04653ab81eb4e07ddfc2c8c2c1036b75247969c05a4f25451cd78913a971f1899d9f2bddec9cf8e0104004f72f20c2856453e5aa3bcd2a8200670ec28feda38f67cc400fc72ef1966956656ec0765478c9d16e9a9e470c07f9ed").expect("expected valid hex");
+        let lock: InstantLock = deserialize(lock_data.as_slice()).expect("expected to deserialize");
+


extract both strings as constants in the tests module (if decode is defined as constant, extract the whole call but i dont expect it to be const tbh)

This is a fix PR following the existing pattern. Should be cleaned up separately, will just add noise.

idk what pattern you are talking about tbh, what ever, not a big deal

Ah haha i though you want to consolidate all other places where those two are used in tests. If thats not it im not sure why you wanted me to move them.

ZocoLini · 2026-02-03T03:02:27Z

dash/src/sml/masternode_list_engine/message_request_verification.rs

+    pub fn is_lock_quorum_lookup_by_quorum_index_not_array_position() {
+        let block_hex =
+            include_str!("../../../tests/data/test_DML_diffs/masternode_list_engine.hex");
+        let data = hex::decode(block_hex).expect("decode hex");


you can also extract this string though

ZocoLini · 2026-02-03T05:27:01Z

dash/src/sml/masternode_list_engine/message_request_verification.rs

+
+        let lock_data = hex::decode("01018d53e7997ead57409750942af0d5e0aafc06f852a9a52308f4781b6a8220298f00000000c6f9d8c63dd15937ea70aaddb7890daad42c91bf6818e2bf76d183d6f2d9215b4b5f84978fad9dde7ab52bdcc0674be891e9029cc1ef0cb01200000000000000a27c98836c4c04653ab81eb4e07ddfc2c8c2c1036b75247969c05a4f25451cd78913a971f1899d9f2bddec9cf8e0104004f72f20c2856453e5aa3bcd2a8200670ec28feda38f67cc400fc72ef1966956656ec0765478c9d16e9a9e470c07f9ed").expect("expected valid hex");
+        let lock: InstantLock = deserialize(lock_data.as_slice()).expect("expected to deserialize");
+


idk what pattern you are talking about tbh, what ever, not a big deal

fix: look up quorum by actual quorum_index not by array position

457f854

ZocoLini requested changes Feb 3, 2026

View reviewed changes

xdustinface requested a review from ZocoLini February 3, 2026 03:15

ZocoLini approved these changes Feb 3, 2026

View reviewed changes

xdustinface merged commit ad3f0e9 into v0.42-dev Feb 3, 2026
51 checks passed

xdustinface deleted the fix/quorum-index-lookup branch February 3, 2026 12:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: look up quorum by actual `quorum_index` not by array position#406

fix: look up quorum by actual `quorum_index` not by array position#406
xdustinface merged 1 commit intov0.42-devfrom
fix/quorum-index-lookup

xdustinface commented Feb 2, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Feb 2, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

ZocoLini Feb 3, 2026

Uh oh!

xdustinface Feb 3, 2026

Uh oh!

ZocoLini Feb 3, 2026

Uh oh!

xdustinface Feb 3, 2026

Uh oh!

ZocoLini Feb 3, 2026

Uh oh!

ZocoLini Feb 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		let lock_data = hex::decode("01018d53e7997ead57409750942af0d5e0aafc06f852a9a52308f4781b6a8220298f00000000c6f9d8c63dd15937ea70aaddb7890daad42c91bf6818e2bf76d183d6f2d9215b4b5f84978fad9dde7ab52bdcc0674be891e9029cc1ef0cb01200000000000000a27c98836c4c04653ab81eb4e07ddfc2c8c2c1036b75247969c05a4f25451cd78913a971f1899d9f2bddec9cf8e0104004f72f20c2856453e5aa3bcd2a8200670ec28feda38f67cc400fc72ef1966956656ec0765478c9d16e9a9e470c07f9ed").expect("expected valid hex");
		let lock: InstantLock = deserialize(lock_data.as_slice()).expect("expected to deserialize");

Conversation

xdustinface commented Feb 2, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

ZocoLini Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

xdustinface Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

ZocoLini Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

xdustinface Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

ZocoLini Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

ZocoLini Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xdustinface commented Feb 2, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Feb 2, 2026 •

edited

Loading