Skip to content

audit: confirm SECONDS_PER_DAY is used consistently across codebase - #708

Open
Horlarmmy wants to merge 2 commits into
Iris-IV:mainfrom
Horlarmmy:hotfix/duplicate-duration-constant
Open

audit: confirm SECONDS_PER_DAY is used consistently across codebase#708
Horlarmmy wants to merge 2 commits into
Iris-IV:mainfrom
Horlarmmy:hotfix/duplicate-duration-constant

Conversation

@Horlarmmy

Copy link
Copy Markdown

Summary

Closes #671

Audits the codebase for ad-hoc 86400 literals and
confirms all campaign-duration-in-days arithmetic goes through the
centralized SECONDS_PER_DAY constant.

Audit findings

Location Pattern Uses SECONDS_PER_DAY?
lifecycle.rs duration_days.checked_mul(crate::SECONDS_PER_DAY) ✅ Direct
campaigns/create.rs Delegates to calculate_deadline(...) ✅ Indirect
campaigns/update.rs additional_days * crate::SECONDS_PER_DAY ✅ Direct
campaigns/update.rs total_duration_seconds / crate::SECONDS_PER_DAY ✅ Direct
campaigns/withdraw.rs delay_days * crate::SECONDS_PER_DAY ✅ Direct

Changes

  • src/constants.rs — Updated docstring to note the constant is
    consistently imported by all callers.
  • src/tests/test_lifecycle.rs — Added three unit tests for
    calculate_deadline: happy path (30 days), zero days, and overflow
    rejection.

Verification

  • cargo fmt --check — pass
  • cargo clippy --all-targets --features testutils -- -D warnings — pass
  • cargo test test_calculate_deadline — 3/3 pass

@drips-wave

drips-wave Bot commented Jul 30, 2026

Copy link
Copy Markdown

@Horlarmmy Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@davidmaronio

Copy link
Copy Markdown
Contributor

nice focused audit. i grepped the repo and the claim checks out: 86400 appears in src/ only in src/constants.rs, everything else (TOKEN_UPDATE_DELAY_SECS etc.) derives from SECONDS_PER_DAY, so the "#671 all callers import these constants" comment is accurate. the three calculate_deadline tests are also correct against the real implementation at src/lifecycle.rs:111: checked_mul overflow maps to ValidationFailed, and zero days returning current_time matches the code.

two small notes:

  1. src/tests/test_lifecycle.rs, test_calculate_deadline_overflow_rejected: u64::MAX / SECONDS_PER_DAY + 1 overflows the multiply, good, but consider also covering the checked_add path (huge current_time plus a valid duration), since that's the second failure branch in calculate_deadline and currently untested.

  2. src/tests/test_lifecycle.rs: the happy-path test computes expected with the same formula as the implementation, so it can't catch a formula bug, only an overflow-handling regression. a hardcoded expected value (1_000_000 + 2_592_000) would make it a real oracle.

the failing test check is the stale-base test_campaign_update 4-tuple thing from main, unrelated to this diff.

gate: this branch is behind main, rebase it and CI should go green, happy to see it merge after that (the two test suggestions above are nice-to-haves, not blockers).

@Horlarmmy

Copy link
Copy Markdown
Author

Thank you for the feedback, All good now

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.

[Contracts] constants.rs SECONDS_PER_DAY is duplicated conceptually by any campaign-duration-in-days arithmetic outside constants.rs

2 participants