fix: compute SML entry hash with SER_GETHASH semantics in dash

[xdustinface]

MasternodeListEntry::calculate_entry_hash hashed the full network consensus serialization, including the leading version. Dash Core's CSimplifiedMNListEntry::CalcHash uses CHashWriter(SER_GETHASH, ...), and SER_GETHASH does not set SER_NETWORK, so the SER_NETWORK-gated leading nVersion is omitted from the hash pre-image. The two contexts therefore diverged for every entry.

Extract the post-version body of the wire encoder into a shared consensus_encode_body helper. consensus_encode writes version then the body (byte-identical to before), while calculate_entry_hash hashes the body alone, matching Core's CalcHash. Verified against Core for both a version 1 and a version 2 Evo entry.

Also fix SocketAddr decoding to use to_ipv4_mapped instead of to_ipv4, so an all-zero (::) service address round-trips to the same 16 bytes instead of acquiring an ::ffff: mapped prefix. The prefix corruption changed the wire bytes and thus the entry hash for masternodes with an unset service.

Summary by CodeRabbit

• Bug Fixes

  • Preserve IPv6 address format during consensus decoding to avoid unintended IPv4-mapping of all-zero addresses.
  • Correct masternode entry hash computation to match expected network compatibility and hashing preimage.

• Tests

  • Added regression tests validating IPv6 address round-trip and masternode list hash/encoding compatibility.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ca2d9d82-59a6-46c2-96b6-a2f78a782e8f

📥 Commits

Reviewing files that changed from the base of the PR and between 458894f and 09a142f.

📒 Files selected for processing (3)

• dash/src/sml/address.rs
• dash/src/sml/masternode_list_entry/hash.rs
• dash/src/sml/masternode_list_entry/mod.rs

🚧 Files skipped from review as they are similar to previous changes (3)

• dash/src/sml/masternode_list_entry/hash.rs
• dash/src/sml/address.rs
• dash/src/sml/masternode_list_entry/mod.rs

---

📝 Walkthrough

Walkthrough

The PR splits MasternodeListEntry consensus encoding into version and body parts, updates entry hashing to use the body-only preimage, and changes SocketAddr decoding to recognize IPv4-mapped IPv6 values while preserving round-trip bytes.

Changes

Consensus Encoding and Hashing Alignment

Layer / File(s)	Summary
MasternodeListEntry encoding refactor: version and body separation dash/src/sml/masternode_list_entry/mod.rs	consensus_encode now writes version first and delegates the remaining fields to consensus_encode_body, which encodes the version-dependent body fields.
Hash calculation: use body encoding as preimage dash/src/sml/masternode_list_entry/hash.rs	calculate_entry_hash now hashes the body-only consensus encoding, and the new fixture-based test checks the resulting hashes against Dash Core values.
SocketAddr decoding: IPv4-mapped IPv6 detection dash/src/sml/address.rs	Consensus decoding now uses Ipv6Addr::to_ipv4_mapped() when deciding whether a 16-byte value should become SocketAddr::V4 or remain SocketAddr::V6; a regression test verifies exact byte preservation for an all-zero payload.

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• dashpay/rust-dashcore#797: Also changes MasternodeListEntry consensus encoding behavior in dash/src/sml/masternode_list_entry/mod.rs, with direct impact on serialized bytes and downstream hashing.

Suggested reviewers

• QuantumExplorer
• ZocoLini

Poem

🐰 I nibbled the bytes, then hopped away,
Version and body learned their roles today.
Hashes now match with a cleaner stride,
IPv6 keeps its form inside.
Hop hop—consensus shines!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately reflects the main change: fixing masternode list entry hash computation to use SER_GETHASH semantics, which is the central objective of the PR.
Docstring Coverage	✅ Passed	Docstring coverage is 90.00% which is sufficient. The required threshold is 80.00%.
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/sml-entry-hash-gethash

---

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/sml/masternode_list_entry/hash.rs (1)

6-10: 💤 Low value

Consider returning Result or documenting infallibility.

The expect("encoding failed") violates the guideline to avoid expect() in library code. However, encoding to a Vec<u8> is practically infallible since Vec's Write impl only fails on allocation exhaustion (which would panic anyway).

Two options:

1. Keep as-is but add a comment explaining why failure is impossible
2. Change signature to return Result<sha256d::Hash, std::io::Error> for API consistency

Given that this is a public method and allocation failure would abort regardless, option 1 is reasonable here.

📝 Suggested documentation

 impl MasternodeListEntry {
     pub fn calculate_entry_hash(&self) -> sha256d::Hash {
+        // Vec<u8> write is infallible (barring OOM which aborts), so expect is safe here.
         let mut writer = Vec::new();
         self.consensus_encode_body(&mut writer).expect("encoding failed");
         sha256d::Hash::hash(&writer)
     }
 }

As per coding guidelines: "Avoid unwrap() and expect() in library code; use proper error types".

🤖 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/sml/masternode_list_entry/hash.rs` around lines 6 - 10, The method
calculate_entry_hash currently calls self.consensus_encode_body(&mut
writer).expect("encoding failed"), which uses expect in library code; replace
this by keeping the current behavior but add a concise comment explaining why
encoding to a Vec<u8> is effectively infallible (the Write impl for Vec only
fails on allocation exhaustion which would abort) and that returning a Result
was considered but omitted for API stability; reference the calculate_entry_hash
and consensus_encode_body symbols and state that sha256d::Hash::hash(&writer)
remains unchanged.

🤖 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/sml/masternode_list_entry/hash.rs`:
- Around line 6-10: The method calculate_entry_hash currently calls
self.consensus_encode_body(&mut writer).expect("encoding failed"), which uses
expect in library code; replace this by keeping the current behavior but add a
concise comment explaining why encoding to a Vec<u8> is effectively infallible
(the Write impl for Vec only fails on allocation exhaustion which would abort)
and that returning a Result was considered but omitted for API stability;
reference the calculate_entry_hash and consensus_encode_body symbols and state
that sha256d::Hash::hash(&writer) remains unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fa716e82-9a61-4a6f-9dd8-5f0fb0b7d223

📥 Commits

Reviewing files that changed from the base of the PR and between a132945 and 458894f.

📒 Files selected for processing (3)

• dash/src/sml/address.rs
• dash/src/sml/masternode_list_entry/hash.rs
• dash/src/sml/masternode_list_entry/mod.rs

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.67%. Comparing base (5b9796a) to head (09a142f).

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #798      +/-   ##
==========================================
+ Coverage   72.61%   72.67%   +0.05%     
==========================================
  Files         323      323              
  Lines       71747    71786      +39     
==========================================
+ Hits        52097    52167      +70     
+ Misses      19650    19619      -31     

Flag	Coverage Δ
core	76.79% <100.00%> (+0.03%)	⬆️
ffi	45.44% <ø> (+0.01%)	⬆️
rpc	20.00% <ø> (ø)
spv	90.35% <ø> (+0.18%)	⬆️
wallet	71.29% <ø> (ø)

Files with missing lines	Coverage Δ
dash/src/sml/address.rs	100.00% <100.00%> (ø)
dash/src/sml/masternode_list_entry/hash.rs	100.00% <100.00%> (ø)
dash/src/sml/masternode_list_entry/mod.rs	62.31% <100.00%> (+2.31%)	⬆️

... and 5 files with indirect coverage changes

[github-actions]

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.