fix(quoter-bot): accept fractional gwei fee bounds - #218
Merged
Merged
Conversation
MAX_FEE_GWEI and PRIORITY_FEE_GWEI parsed as integers, so 1 gwei was the lowest tip an operator could set. Base's going tip is ~0.005 gwei, and the Base maker paid the 200x difference on every publication until the wallet ran down and the bot halted. Parse both through viem parseGwei with a >=1 wei floor, matching what midnight-liquidation already does, and relax the YAML gate to match. The values are bigint wei downstream, so bumping and policy ceilings are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
parseGwei rounds half-up past nine decimal places, so MAX_FEE_GWEI 1.0000000005 resolved to one wei ABOVE the ceiling its operator declared, and a sub-wei tip rounded up instead of being rejected. Cap the decimal regex at nine places on both the env and YAML paths so the conversion is exact. Also note on MAX_FEE_GWEI that a ceiling near twice the base fee clamps the first send and makes the first bump drop — newly reachable on Base now that the ceiling can go below a gwei. BOTS-136 covers guarding it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
haydenshively
marked this pull request as ready for review
September 14, 2026 16:03
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
cashd
approved these changes
Sep 14, 2026
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
Collaborator
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
2 tasks
haydenshively
added a commit
that referenced
this pull request
Sep 15, 2026
`pr-describe` made the agent measure the diff by hand — a `git diff --numstat` + awk pipeline plus `tokei` — and paste the result into the PR body under `## Size`. That number was wrong the moment another commit landed, and nothing corrected it. CI now owns the measurement and keeps one pinned comment per PR, recomputed on every push. ## Changes - **`packages/ci-scripts/src/pr-size/`** — `summary.ts` holds the pure logic (bucketing, tokei-totals validation, table rendering); `orchestrate/comment.ts` reads the PR files API and upserts a marker-tagged comment. Reuses the package's existing `githubApi`/`paginate`/`readPayload` helpers rather than adding a dependency. - **`.github/workflows/pr-size.yml`** — two jobs. `Comment` holds `pull-requests: write`, checks out `main`, and never touches PR code: GitHub's PR files API serves the per-file additions/deletions the table needs, so no checkout of the diff is required at all. `Measure` checks out PR head for `tokei` but holds no write token, and hands over three integers through a job output. - **`.claude/commands/pr-describe.md`** — Step 1.2 and the `## Size` section deleted; the agent no longer measures. Also repairs the unbalanced code fences in the Step 3 template, which were pre-existing. - Tests, `knip.json` entry, `README`, `docs/INDEX.md`. ## Notes for review **The port is verified, not asserted.** Run over a 229-file range spanning all four buckets, the new renderer produces a table byte-identical to the old awk. Against live PR #218, the rendered totals match GitHub's own `additions`/`deletions`/`changed_files` exactly. **tokei is built from source, pinned to v15.0.0.** It has shipped no prebuilt binary since v12.1.2 in 2021, so pinning one only gets further out of date. `ubuntu-latest` already carries a Rust toolchain, and the cold build measured **45s** in CI on this PR; only the resulting binary is cached, so restore is near instant and a stale entry just rebuilds. Verified that v12.1.2, v14.0.0 and v15.0.0 return identical code/comment/blank totals on the same inputs. The workflow parses tokei's plain-text `Total` row rather than `--output json`, which is stable across all three versions. **Two deliberate divergences from the old pipeline**, both named in comments: a rename's content delta is attributed to the new path, and a binary file counts as one changed file at +0/−0 where the old awk dropped it from the file count entirely. **Cosmetic means cosmetic.** Both jobs carry `continue-on-error`, so a failed build or merge-base leaves the PR green and the comment still posts, saying the code/prose split is unavailable. **`Comment` fails on this PR, by design.** It runs *main's* `ci-scripts`, which has no `pr-size-comment` script until this merges. The workflow run still reports success. It passes from the next PR onward; the writer path was verified end to end by running the orchestrator against this PR directly (created, then updated in place, exactly one comment). ## Deferred [BOTS-138](https://linear.app/morpho-labs/issue/BOTS-138/cirepo-decide-the-trust-boundary-for-pr-request-workflows-holding) — `pull_request` runs the workflow as the PR defines it, so a branch can edit the job holding the write token. That grants a contributor nothing they lack here (pushing a branch already means repo write) and a fork's token is read-only, but it is not the boundary `pull_request_target` would give. `pr-release-label-sync.yml` has the identical exposure, so it is a repo-wide call rather than one to make in this PR. This workflow's header now states the exposure instead of claiming a boundary it does not have. ## Test Plan - [ ] On this PR, confirm the size comment's numbers match `git diff --numstat origin/main...HEAD` - [ ] After merge, confirm the next PR gets the comment from CI and that it is edited in place on a second push 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The Base maker's gas wallet ran down to 0.0043 ETH and the bot halted.
PRIORITY_FEE_GWEIwas setto
1, but Base's going tip is ~0.005 gwei — ~200x the market rate on every publication andcancellation, on a 60-second loop. It could not be set lower: both fee bounds parsed as positive
integers, so
1was the floor.midnight-liquidationalready parses decimal gwei. quoter-bot was the outlier.What
Funds-at-risk — touches fee bounds and the signing-policy ceilings that gate every transaction.
positiveGweiWeiValueinconfig.utils.ts: decimal regex capped at nine places, viemparseGwei, and a >=1 wei floor.MAX_FEE_GWEIandPRIORITY_FEE_GWEIuse it; the other twelveinteger env vars keep
positiveBigIntValueuntouched. The nine-place cap is load-bearing —parseGweirounds half-up beyond it, soMAX_FEE_GWEI=1.0000000005would otherwise resolve onewei above the ceiling its operator declared.
yamlIntegerEnvironmentKeysto a newyamlDecimalEnvironmentKeys.Both
priorityFeeGwei: 0.005and'0.005'load; exponent notation fails loud asinvalid-decimal, becauseparseGweithrows its own untyped error on it.bigintwei onQuoterWritePolicy, and bot-kit'sinitialFees/bumpFees/hasBumpHeadroomare wei-granular with a+1 weibump floor.PRIORITY_FEE_GWEI=1, which is where the 200x came from.Now
0.01, with a per-chain calibration note.Review
Codex (GPT Sol, high reasoning) reviewed the first commit. It found the
parseGweirounding above,fixed in 84b4be4. Its other two findings are one pre-existing root cause — config validates fee and
spend bounds against nominal values that do not hold at send time, so a combination that can never
admit a send passes validation. Deferred as
BOTS-136:
the real guard needs the runtime base fee, which is a send-path change wanting its own review. This
PR mitigates the newly reachable half with an operator-facing note on
MAX_FEE_GWEI.Not in scope
Deriving the tip from
eth_maxPriorityFeePerGasand demotingPRIORITY_FEE_GWEIto a ceiling. Thatis the durable fix for "a hand-set constant silently drains the wallet over days" — nothing today
compares the configured tip to the market. It changes first-send behaviour on a funds-at-risk path
and wants its own TIB rather than riding along with a parser fix the operator is waiting on.
Verified
typecheck,lint,format,knipclean. Root suite: 2942 passed. The two failures aremidnight-liquidation fork suites missing
RPC_URL_8453; the quoter-bot fork e2e fails identically ona pristine
origin/mainworktree (same0x5e71e7fbrevert), so both are environmental.New coverage in
test/config/config.service.test.tsandtest/config/config-loading.test.ts:decimal parses to exact wei, sub-wei and exponent and signed values reject with stable reasons, a
decimal pair with no bump headroom still reports
incoherent-bounds, and a field that must stayintegral still rejects a decimal.
🤖 Generated with Claude Code