fix: write compact-size count in QuorumSnapshot encode

[xdustinface]

QuorumSnapshot::consensus_encode wrote the active-members bitset without the leading compact-size bit count that consensus_decode reads (and that Dash Core's DYNBITSET(activeQuorumMembers) serializes), so encode and decode were asymmetric and any re-serialized QuorumSnapshot desynced on decode.

This never surfaced because the only consensus-encode path for the type is serializing an outgoing QRInfo response (message.rs), which an SPV client never produces, and the only existing test was decode-only. Adds a round-trip test.

Summary by CodeRabbit

• Bug Fixes

  • Enhanced consensus message encoding/decoding for improved data serialization reliability.

• Tests

  • Added roundtrip testing to verify message encoding/decoding correctness.

[coderabbitai]

📝 Walkthrough

Walkthrough

Updated QuorumSnapshot serialization in message_qrinfo.rs by prepending a compact-size length to the fixed bitset during encoding, added corresponding imports for bitset helpers, and verified the roundtrip with a new unit test.

Changes

QuorumSnapshot Encoding Fix

Layer / File(s)	Summary
QuorumSnapshot encoding logic and test dash/src/network/message_qrinfo.rs	Imports for write_compact_size, read_compact_size, write_fixed_bitset, and read_fixed_bitset are added. QuorumSnapshot::consensus_encode is updated to write the bitset length as a compact-size value before encoding the fixed bitset. A quorum_snapshot_encode_decode_roundtrip unit test constructs a snapshot, serializes and deserializes it, and asserts equality.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A snapshot's bits now pack with grace,
Compact-size lengths find their place,
The roundtrip test hops through encode,
Bitsets aligned, no stone unturned, to revoke,
Serialization flows just right! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: adding a compact-size count write operation to the QuorumSnapshot encode method, which directly addresses the encoding asymmetry detailed in the PR objectives.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/quorum-snapshot-encode-compact-size

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

dash/src/network/message_qrinfo.rs (1)

319-320: ⚡ Quick win

Pin the wire format in the test.

Roundtrip covers internal symmetry, but it would still pass if encode and decode drifted together. Please also assert that the serialized bytes include the compact-size member count before the bitset so this regression stays directly covered.

🧪 Proposed test hardening

         let bytes = serialize(&snapshot);
+        assert_eq!(
+            &bytes[..5],
+            &[1, 0, 0, 0, 10],
+            "wire format must include the active_quorum_members compact-size count",
+        );
         let decoded: QuorumSnapshot = deserialize(&bytes).expect("deserialize QuorumSnapshot");

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dash/src/network/message_qrinfo.rs` around lines 319 - 320, The current test
only roundtrips via serialize/deserialize (serialize(&snapshot) and
deserialize(&bytes)), but to pin the wire format you must also assert the
serialized bytes contain the compact-size encoded member count immediately
before the bitset; compute the compact-size encoding for snapshot.members.len()
(same encoding used elsewhere in this module), then after calling
serialize(&snapshot) assert that the byte sequence for that compact-size integer
appears at the expected location right before the bitset payload in bytes (or
assert bytes.windows(n).position(...) yields an index such that the subsequent
bytes match the bitset), so the test fails if the member-count encoding or
ordering drifts.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@dash/src/network/message_qrinfo.rs`:
- Around line 319-320: The current test only roundtrips via
serialize/deserialize (serialize(&snapshot) and deserialize(&bytes)), but to pin
the wire format you must also assert the serialized bytes contain the
compact-size encoded member count immediately before the bitset; compute the
compact-size encoding for snapshot.members.len() (same encoding used elsewhere
in this module), then after calling serialize(&snapshot) assert that the byte
sequence for that compact-size integer appears at the expected location right
before the bitset payload in bytes (or assert bytes.windows(n).position(...)
yields an index such that the subsequent bytes match the bitset), so the test
fails if the member-count encoding or ordering drifts.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 861b72d3-9bda-4086-a808-d3f74e619a77

📥 Commits

Reviewing files that changed from the base of the PR and between 3d0d5dc and 98f8b43.

📒 Files selected for processing (1)

• dash/src/network/message_qrinfo.rs

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.56%. Comparing base (3d0d5dc) to head (98f8b43).

Additional details and impacted files

@@           Coverage Diff           @@
##              dev     #796   +/-   ##
=======================================
  Coverage   72.55%   72.56%           
=======================================
  Files         322      322           
  Lines       71006    71019   +13     
=======================================
+ Hits        51518    51532   +14     
+ Misses      19488    19487    -1     

Flag	Coverage Δ
core	76.54% <100.00%> (+0.07%)	⬆️
ffi	46.42% <ø> (ø)
rpc	20.00% <ø> (ø)
spv	90.15% <ø> (-0.08%)	⬇️
wallet	71.29% <ø> (ø)

Files with missing lines	Coverage Δ
dash/src/network/message_qrinfo.rs	52.56% <100.00%> (+14.10%)	⬆️

... and 3 files with indirect coverage changes