Skip to content

fix(RateChangeQueue): empty queue behavior#272

Open
wjmelements wants to merge 7 commits intomainfrom
fix-266
Open

fix(RateChangeQueue): empty queue behavior#272
wjmelements wants to merge 7 commits intomainfrom
fix-266

Conversation

@wjmelements
Copy link
Collaborator

@wjmelements wjmelements commented Jan 28, 2026

Fixes #266
Reviewer @ZenGround0

Changes

  • move reset of head = 0 to when zeroing rail
  • change string revert error 'Queue is empty' into abi 'error EmptyQueue()'
  • remove redundant bounds checking with unchecked

@FilOzzy FilOzzy added this to FOC Jan 28, 2026
@github-project-automation github-project-automation bot moved this to 📌 Triage in FOC Jan 28, 2026
@ZenGround0
Copy link
Contributor

I just beat you to posting the PR and I kind of like my/claude's solution better ( https://github.com/FilOzone/filecoin-pay/pull/269/changes) but I'm happy to talk about it more.

@wjmelements
Copy link
Collaborator Author

The test failure is unrelated and was broken in a dependency. I will try to fix it in a separate PR.

Run forge coverage --report lcov --ir-minimum
  forge coverage --report lcov --ir-minimum
  shell: /usr/bin/bash -e {0}
  env:
    FOUNDRY_PROFILE: ci
Warning: This is a nightly build of Foundry. It is recommended to use the latest stable version. To mute this warning set `FOUNDRY_DISABLE_NIGHTLY_WARNING` in your environment. 

Error: failed to extract foundry config:
foundry config error: selected profile `ci` does not exist
Error: Process completed with exit code 1.

@ZenGround0
Copy link
Contributor

Looks like we could take the last two bullet points as nice add ons in addition to PR #269

@wjmelements
Copy link
Collaborator Author

I just beat you to posting the PR and I kind of like my/claude's solution better ( https://github.com/FilOzone/filecoin-pay/pull/269/changes) but I'm happy to talk about it more.

Discuss on the other issue: #266 (comment)

@wjmelements
Copy link
Collaborator Author

The test failure is unrelated and was broken in a dependency. I will try to fix it in a separate PR.

Run forge coverage --report lcov --ir-minimum
  forge coverage --report lcov --ir-minimum
  shell: /usr/bin/bash -e {0}
  env:
    FOUNDRY_PROFILE: ci
Warning: This is a nightly build of Foundry. It is recommended to use the latest stable version. To mute this warning set `FOUNDRY_DISABLE_NIGHTLY_WARNING` in your environment. 

Error: failed to extract foundry config:
foundry config error: selected profile `ci` does not exist
Error: Process completed with exit code 1.

#273

@codecov
Copy link

codecov bot commented Jan 28, 2026

Codecov Report

❌ Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/RateChangeQueue.sol 71.42% 4 Missing ⚠️

❌ Your patch check has failed because the patch coverage (73.33%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #272   +/-   ##
=======================================
  Coverage        ?   76.99%           
=======================================
  Files           ?        5           
  Lines           ?      539           
  Branches        ?      104           
=======================================
  Hits            ?      415           
  Misses          ?      120           
  Partials        ?        4           
Flag Coverage Δ
foundry 76.99% <73.33%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/FilecoinPayV1.sol 83.22% <100.00%> (ø)
src/RateChangeQueue.sol 80.95% <71.42%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wjmelements
Copy link
Collaborator Author

Regretfully I updated my forge to 1.5.1 and now I cannot downgrade to 1.3.5. I have tried foundryup -C 9979a41b5daa5da1572d973d7ac5a3dd2afc0221

@wjmelements
Copy link
Collaborator Author

I have reviewed the lines codecov thinks are missing test coverage. They all have test coverage. Likely related to via-ir.

@BigLep BigLep added this to the M4.5: GA Fast Follows milestone Jan 30, 2026
@rjan90 rjan90 moved this from 📌 Triage to 🔎 Awaiting review in FOC Jan 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🔎 Awaiting review
Status: No status

Development

Successfully merging this pull request may close these issues.

Audit Fix 1: RateChangeQueue clearing behavior

4 participants