Skip to content

Fix double recalculate arbitrage - #17

Open
Nov1kov wants to merge 2 commits into
BowTiedDevil:mainfrom
Nov1kov:fix_double_recalculate_arbitrage
Open

Nov1kov wants to merge 2 commits into
BowTiedDevil:mainfrom
Nov1kov:fix_double_recalculate_arbitrage

Conversation

@Nov1kov

@Nov1kov Nov1kov commented Jan 13, 2024

Copy link
Copy Markdown

According to this fix, 84dd8d4

There are supposing the:

  • calculate, calculate_arbitrage, calculate_with_pool must have the self._pre_calculation_check(override_state)
  • _calculate mustn't have the _pre_calculation_check

BowTiedDevil added a commit that referenced this pull request Jun 20, 2026
V3-V4-V3 reverted on mainnet with CurrencyNotSettled() (permutation row #17,
~50% simulatable). The inner V4 swap used a static solver amount
(V4_SWAP_COMPACT(out_a)) and the V4 output was taken with a static
V4_TAKE_COMPACT(out_b). V3's optimistic transfer delivers V3a's forward_a to
the PoolManager before V3a's callback runs the V4 unlock, and the V4 swap
computes its actual forward_b on-chain — so whenever the on-chain amounts
differ from the solver's out_a/out_b (common under real price impact), a
residual PM delta survived at unlock-end, causing CurrencyNotSettled.

Switch to V4_SWAP_DYNAMIC (consume the actual settled forward_a delta via PM
exttload) + V4_TAKE_DELTA (take the full actual forward_b output) +
V4_SETTLE_ALL (sweep dust). The nesting is unchanged — V3's optimistic
transfer already delivers forward_a before the callback, so the bug was the
static amounts, not the nesting. Mirrors the V2-V2-V4 fix (commit 2e50553).

Validation: structural regression test in tests/arbitrage/test_v3_v4_v3_encoder.py
pins the new ordering. On-chain Ape+Foundry regression (executor repo) forces V4
over-production and proves the OLD stream reverts with CurrencyNotSettled while
the NEW stream succeeds with profit, confirming the root cause and the fix.

Plan: B2TSXX
BowTiedDevil added a commit that referenced this pull request Jun 21, 2026
V3-V4-V3 reverted on mainnet with CurrencyNotSettled() (row #17,
~50% simulatable). The inner V4 swap used a static solver amount
(V4_SWAP_COMPACT(out_a)) and the V4 output was taken with a static
V4_TAKE_COMPACT(out_b). V3's optimistic transfer delivers V3a's
forward_a to the PoolManager before V3a's callback runs the V4
unlock, and the V4 swap computes its actual forward_b on-chain, so
whenever on-chain amounts differ from the solver's out_a/out_b
(common under price impact) a residual PM delta survives at
unlock-end, causing CurrencyNotSettled.

Switch to V4_SWAP_DYNAMIC (consume the actual settled forward_a
delta via PM exttload) plus V4_TAKE_DELTA (take the full actual
forward_b output) plus V4_SETTLE_ALL (sweep dust). Nesting is
unchanged: V3's optimistic transfer already delivers forward_a
before the callback, so the bug was the static amounts, not the
nesting. Mirrors V2-V2-V4 (commit 2e50553).

A structural regression test pins the new ordering, and an
on-chain Ape+Foundry regression forces V4 over-production to prove
the OLD stream reverts with CurrencyNotSettled while the NEW
stream succeeds with profit.

Plan: B2TSXX
BowTiedDevil added a commit that referenced this pull request Jul 21, 2026
V3-V4-V3 reverted on mainnet with CurrencyNotSettled() (row #17,
~50% simulatable). The inner V4 swap used a static solver amount
(V4_SWAP_COMPACT(out_a)) and the V4 output was taken with a static
V4_TAKE_COMPACT(out_b). V3's optimistic transfer delivers V3a's
forward_a to the PoolManager before V3a's callback runs the V4
unlock, and the V4 swap computes its actual forward_b on-chain, so
whenever on-chain amounts differ from the solver's out_a/out_b
(common under price impact) a residual PM delta survives at
unlock-end, causing CurrencyNotSettled.

Switch to V4_SWAP_DYNAMIC (consume the actual settled forward_a
delta via PM exttload) plus V4_TAKE_DELTA (take the full actual
forward_b output) plus V4_SETTLE_ALL (sweep dust). Nesting is
unchanged: V3's optimistic transfer already delivers forward_a
before the callback, so the bug was the static amounts, not the
nesting. Mirrors V2-V2-V4 (commit 2e50553).

A structural regression test pins the new ordering, and an
on-chain Ape+Foundry regression forces V4 over-production to prove
the OLD stream reverts with CurrencyNotSettled while the NEW
stream succeeds with profit.

Plan: B2TSXX
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.

1 participant