Skip to content

refactor: miscellaneous type safety and lookup improvements#7136

Merged
PastaPastaPasta merged 3 commits intodashpay:developfrom
PastaPastaPasta:refac-misc
Feb 13, 2026
Merged

refactor: miscellaneous type safety and lookup improvements#7136
PastaPastaPasta merged 3 commits intodashpay:developfrom
PastaPastaPasta:refac-misc

Conversation

@PastaPastaPasta
Copy link
Member

Issue being fixed or feature implemented

  • Use size_t consistently in VarInt bitset serialization, replacing mixed int/unsigned int
    types
    • Use CCoinsViewMemPool in IsCollateralValid to resolve inputs from both the UTXO set and
      mempool, removing manual mempool iteration
    • Introduce GetMemberAtIndex(size_t) for safe index-based DKG member lookup with nullptr
      return, matching the existing GetMember(const uint256&) API, and add assertions at call sites
      where PreVerifyMessage guarantees validity

What was done?

see commits

How Has This Been Tested?

Breaking Changes

N/A

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

PastaPastaPasta and others added 3 commits February 12, 2026 16:38
ReadFixedBitSet/WriteFixedBitSet use size_t throughout, but their VarInt
counterparts use int32_t for index tracking. This limits VarInt bitsets
to 2^31 entries and involves mixed signed/unsigned arithmetic. Modernize
to match the size_t convention using std::optional to replace the
sentinel value of -1.

Wire format is preserved: when last is unset (was -1), i - (-1) == i + 1,
so the replacement expression produces identical values.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tion

IsCollateralValid manually looks up inputs from both the mempool and
UTXO set using two separate code paths. The sibling function
IsValidInOuts already uses CCoinsViewMemPool which unifies both lookups
behind a single interface. Align the two functions by adopting the same
pattern here.

This also allows merging the cs_main lock scope to cover both the coin
lookup and the ATMPIfSaneFee call, removing a redundant lock acquisition.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ion handling

GetMember(const uint256&) provides safe hash-based member lookup
returning nullptr when not found. However, index-based access throughout
the justification code uses raw members[index] with no equivalent
safety. Add GetMemberAtIndex(size_t) as the index-based counterpart and
adopt it in PreVerifyMessage/ReceiveMessage for CDKGJustification,
alongside structured binding modernization.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@PastaPastaPasta PastaPastaPasta added this to the 23.1 milestone Feb 13, 2026
@github-actions
Copy link

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

Walkthrough

This pull request refactors multiple core components to improve safety and consistency. The collateral validation logic is reworked to use a mempool-aware coin view with explicit synchronization, replacing previous per-input checks. DKG session member access patterns are strengthened with structured bindings and a new GetMemberAtIndex helper method to safely handle out-of-bounds cases. Bitset serialization logic is updated to use std::optional<size_t> for index tracking, replacing int-based logic and adding an explicit stopper VarInt to terminate sequences.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (21 files):

⚔️ .github/workflows/build-depends.yml (content)
⚔️ src/coinjoin/coinjoin.cpp (content)
⚔️ src/interfaces/node.h (content)
⚔️ src/interfaces/wallet.h (content)
⚔️ src/llmq/dkgsession.cpp (content)
⚔️ src/llmq/dkgsession.h (content)
⚔️ src/net.cpp (content)
⚔️ src/net.h (content)
⚔️ src/node/interfaces.cpp (content)
⚔️ src/qt/bitcoingui.cpp (content)
⚔️ src/qt/governancelist.cpp (content)
⚔️ src/qt/masternodelist.cpp (content)
⚔️ src/qt/masternodelist.h (content)
⚔️ src/qt/masternodemodel.cpp (content)
⚔️ src/qt/masternodemodel.h (content)
⚔️ src/qt/proposalmodel.cpp (content)
⚔️ src/qt/proposalmodel.h (content)
⚔️ src/qt/proposalwizard.cpp (content)
⚔️ src/qt/proposalwizard.h (content)
⚔️ src/serialize.h (content)
⚔️ src/wallet/interfaces.cpp (content)

These conflicts must be resolved before merging into develop.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: type safety improvements (size_t usage), safe lookup patterns (GetMemberAtIndex), and VarInt bitset refactoring.
Description check ✅ Passed The description clearly relates to the changeset, detailing specific improvements to type safety, coin view handling, and member lookup with rationale and scope.

✏️ 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
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch refac-misc
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉


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.

Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm 590236a

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 590236a

@PastaPastaPasta PastaPastaPasta merged commit 2c7ef1d into dashpay:develop Feb 13, 2026
79 of 84 checks passed
@PastaPastaPasta PastaPastaPasta deleted the refac-misc branch February 13, 2026 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants