Skip to content

CFeeRate internal arithmetic overflows int64_t for fees near MAX_MONEY #445

Description

@JSanchezFDZ

Summary

CFeeRate's two-arg constructor and GetFee() perform internal arithmetic that overflows int64_t for fee amounts anywhere near Raptoreum's MAX_MONEY (21B * COIN = 2.1e18). The constructor in src/policy/feerate.cpp computes:

CFeeRate::CFeeRate(const CAmount &nFeePaid, size_t nBytes_) {
    ...
    if (nSize > 0)
        nSatoshisPerK = nFeePaid * 1000 / nSize;   // <- overflows for nFeePaid > int64_t::max / 1000
    ...
}

int64_t::max() / 1000 ≈ 9.2e15, which is ~228x smaller than MAX_MONEY. So any fee amount within roughly the upper 99.5% of the valid CAmount range overflows here, with undefined behaviour. Under signed-overflow sanitizers / -ftrapv builds, this aborts the process.

This was inherited from upstream Bitcoin, where MAX_MONEY = 21M * COIN ≈ 2.1e15, and MAX_MONEY * 1000 ≈ 2.1e18 fits comfortably in int64_t. The 1000x bump to Raptoreum's cap didn't get reflected in CFeeRate.

Real-world impact today is small (no caller passes single-tx fees that large), but:

  • It surfaced as a test-suite cascade — see PR test(amount): fix GetFeeTest int64 overflow causing 318-failure cascade #444, which fixes the specific test boundary case by switching to OLD_MAX_MONEY. That's a minimal workaround, not a real fix to CFeeRate itself.
  • Defensive arithmetic for consensus-adjacent code is cheap; UB lurking near MAX_MONEY is exactly the kind of thing that becomes a footgun later (e.g. when fuzz-testing or sanitizer CI lands).

Proposed fix

Promote internal arithmetic in CFeeRate::CFeeRate(CAmount, size_t) and CFeeRate::GetFee(size_t) to 128-bit. Two natural options:

  1. __int128 — zero dependencies, supported by GCC/Clang on every platform Raptoreum already targets. Cheapest patch.
  2. boost::multiprecision::int128_t / CAmount128 — already used in the codebase for asset balances (src/amount.h:15). More portable and consistent with existing conventions.

Either way, the public API stays int64_t (CAmount nSatoshisPerK), so only the intermediate multiplication moves to 128-bit. Saturation/clamping behaviour at the API edge can match how CAmount128 is handled elsewhere (AssetsRange etc.).

Suggested test additions

Out of scope for this issue

  • The matching question for CTxMemPool::GetMinFee() and other fee-arithmetic call sites — those should be audited as a follow-up once CFeeRate is hardened, but they're a separate ticket.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions