Skip to content

fix(dpp): correct misleading non-mainnet minimum-interval error message#3668

Merged
lklimek merged 1 commit into
v3.1-devfrom
fix/dpp-min-interval-error-message-v3.1-dev
May 19, 2026
Merged

fix(dpp): correct misleading non-mainnet minimum-interval error message#3668
lklimek merged 1 commit into
v3.1-devfrom
fix/dpp-min-interval-error-message-v3.1-dev

Conversation

@lklimek
Copy link
Copy Markdown
Contributor

@lklimek lklimek commented May 19, 2026

Issue

InvalidTokenDistributionBlockIntervalTooShortError's Display literal asserted Minimum allowed is 100. That number is correct only on mainnet. The actual enforced floor is network-dependent (mainnet 100 / testnet 5 / devnet 2 / regtest 1), computed by the validator at packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/validation/v0/mod.rs. On testnet/devnet/regtest the message lied — and it is surfaced verbatim to JS/wasm consumers via generic_consensus_error!.

Fix (Option A — string-only Display reword)

Single line, single file: the #[error(...)] literal now reads

BlockBasedDistribution interval is too short: {interval}. It is below the network-required minimum.

No hardcoded number, no per-network table baked into the message — the validator stays the single source of truth.

Why Option A, not Option B (carry the real minimum)

Option B requires adding a minimum field to the error struct. That struct is bincode::Encode/Decode + PlatformSerialize/PlatformDeserialize, #[platform_serialize(unversioned)], wire-encoded inside ConsensusError (state-transition results / proofs). bincode encodes structs as ordered fieldless concatenation, so a field addition changes the byte layout and Decode shape with no version discriminant to gate it — a hard, ungated breaking consensus-serialization change, plus mandatory regen of every bincode/proof vector carrying this variant. Disproportionate for a cosmetic diagnostic-string bug.

The #[error(...)] literal is pure runtime Display — never serialized. Reword = zero wire impact: no struct/field/derive/serde change, error code 10272 unchanged, validator unchanged, wasm signature unchanged (JS message auto-fixed via passthrough), no vector regen. Non-breaking → plain fix, no !, no BREAKING CHANGE: footer.

Scope / out of scope

  • Touched: 1 file, 1 line. No test asserted the old 100 substring, so no test changed.
  • Sibling InvalidTokenDistributionTimeIntervalTooShortError (Minimum allowed is 3,600,000 ms (1 hour)) is the same network-dependent class but is intentionally out of scope for this minimal fix — flagged here, not expanding scope.

Validation

  • cargo check -p dpp clean (only pre-existing unrelated baseline warnings).
  • cargo clippy -p dpp -- -D warnings: no new findings — the failures are pre-existing baseline in serialization_traits.rs, proven identical on untouched origin/v3.1-dev.
  • cargo test -p dpp token_perpetual_distribution: 405 passed, 0 failed.
  • cargo check -p wasm-dpp: clean — message passthrough builds unchanged.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved error messaging for token distribution block interval validation to dynamically report the network-required minimum value instead of a hardcoded threshold, providing clearer feedback to users when validation fails.

Review Change Stack

The literal said "Minimum allowed is 100" but the enforced floor is
network-dependent (the validator is the single source of truth).
Display-only string reword; non-breaking, no consensus-format change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a1351913-3b97-4f14-a942-31ce924c7119

📥 Commits

Reviewing files that changed from the base of the PR and between 20ed25c and 51b73ec.

📒 Files selected for processing (1)
  • packages/rs-dpp/src/errors/consensus/basic/token/invalid_token_distribution_block_interval_too_short_error.rs

📝 Walkthrough

Walkthrough

Error message for token distribution block interval validation is updated to replace the hardcoded minimum threshold text with a message indicating the interval is below the network-required minimum, making the error more descriptive and contextual.

Changes

Token Distribution Error Message Update

Layer / File(s) Summary
Error message update for block interval validation
packages/rs-dpp/src/errors/consensus/basic/token/invalid_token_distribution_block_interval_too_short_error.rs
The #[error(...)] attribute message is updated to remove the hardcoded "Minimum allowed is 100." wording and instead describe the interval as below the network-required minimum.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

A rabbit hops through the codebase so bright,
Updating an error message—now that's done right!
No "hundred" hardcoded, just network's best way,
A cleaner, smarter error for each network day! 🐰✨

🚥 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 title accurately summarizes the main change: correcting a misleading error message that was network-dependent but displayed a hardcoded minimum value.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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/dpp-min-interval-error-message-v3.1-dev

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.

@github-actions github-actions Bot added this to the v3.1.0 milestone May 19, 2026
@lklimek lklimek marked this pull request as ready for review May 19, 2026 10:06
@lklimek lklimek requested a review from QuantumExplorer as a code owner May 19, 2026 10:06
@thepastaclaw
Copy link
Copy Markdown
Collaborator

thepastaclaw commented May 19, 2026

🕓 Ready for review — next in queue (commit 51b73ec)
Queue position: 1/1

@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.88%. Comparing base (20ed25c) to head (51b73ec).

Additional details and impacted files
@@             Coverage Diff              @@
##           v3.1-dev    #3668      +/-   ##
============================================
- Coverage     88.05%   87.88%   -0.18%     
============================================
  Files          2521     2537      +16     
  Lines        308781   311090    +2309     
============================================
+ Hits         271908   273403    +1495     
- Misses        36873    37687     +814     
Components Coverage Δ
dpp 88.01% <ø> (ø)
drive 87.03% <ø> (ø)
drive-abci 90.05% <ø> (ø)
sdk ∅ <ø> (∅)
dapi-client ∅ <ø> (∅)
platform-version ∅ <ø> (∅)
platform-value 92.17% <ø> (ø)
platform-wallet ∅ <ø> (∅)
drive-proof-verifier 53.13% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lklimek lklimek merged commit ec1b288 into v3.1-dev May 19, 2026
47 checks passed
@lklimek lklimek deleted the fix/dpp-min-interval-error-message-v3.1-dev branch May 19, 2026 10:21
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.

2 participants