GeezerVison

Codecov Report

❌ Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.78%. Comparing base (d706627) to head (2637937).
⚠️ Report is 1 commits behind head on master.

@@            Coverage Diff             @@
##           master     #495      +/-   ##
==========================================
- Coverage   62.08%   60.78%   -1.31%     
==========================================
  Files          14       14              
  Lines        4302     4302              
==========================================
- Hits         2671     2615      -56     
- Misses       1631     1687      +56     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Confidence Score: 3/5

[Medium impact] The changes affect async price and v3 pool-value queries.

Fix the low-price quotes and spacing-one TVL calculations before merging.

Findings

1.  Low prices become zero ▶
2.  Pool balances are wrong ▶

Summary

Reverts the async price and odd-tick TVL fixes and deletes their offline regression tests.

• Async v3 and v4 prices now discard low bits before decimal scaling.
• Async v3 TVL now floors odd tick halves when it builds price bounds.

_{Reviews (1) · Last reviewed commit: "Revert "fix(async): port price-floor and..." · Reviewed by Greptile}

Greptile's review correctly flags two P1 bugs this revert would re-introduce: low prices returning zero and wrong pool TVL balances in the async clients. Those are the exact bugs #493 fixed, and the offline regression tests it added would be deleted.

The failing v3 CI tests that prompted this (DAI-USDC pool params changed on mainnet) are addressed separately in #494 and are not related to the async price/TVL fixes.

Could you clarify what specific problem you're trying to solve with this revert? If there's a new regression from #493 I'm not seeing, happy to fix it directly rather than lose the price-floor and odd-tick TVL corrections.

It's not that I'm gonna roll back, rather opposite

I'm trying to localize a very specific issue

Quick breakdown: codebase was unaffected since the last major PR, everything was green. Then you found that arithmetic problem and made a sync PR, still green (which make sense). Erik merged that, still green after the merge. Then you pushed an async PR for the very same reason, still green. I checked, everything was fine, I merged as well. And that merge broke tests for both sync and async versions, which is insane

At the moment I'm sure: this is infra issue, not ours. Still pondering though what part of infra is broken

Got it — you're using the revert as a diagnostic control, not proposing to drop the fixes. I misread that intent.

The useful result is that the revert's v3 run still fails in both clients: mint_liquidity gets execution reverted: STF, then test_close_position indexes an empty position list. The latter is a downstream failure, not a separate position-closing regression. That narrows the investigation, but doesn't yet identify which infrastructure component is responsible.

I also need to correct my earlier advice on #493: these tests start local Anvil forks, so do not fund or approve anything on mainnet. mint_liquidity already checks both balances and submits both PositionManager approvals before the failing mint; those checks passed in this run. My claim that replenishing a wallet or retrying CI would solve it was not established. Likewise, changing the fee tier in #494 is not proof that the pool moved or that we've found the cause.

The next isolating experiment I'd use is the last-green revision versus the failing revision with the same fork block, Anvil binary and RPC endpoint, each on a fresh fork. The fixture currently leaves the fork block unpinned, and this failing run installed Anvil 1.9.0 (6056c584). At the mint, capture the actual ERC-20 transferFrom failure with a trace and read both allowances to the PositionManager after the approval receipts. Then vary only the RPC endpoint or Anvil version. That separates moving chain state, fork/provider behaviour and transaction handling without conflating them with the arithmetic patch. I haven't reproduced that controlled comparison yet, so I wouldn't call a particular provider or Foundry version the culprit.