test(contracts): CharonLiquidator fork-test suite across 5 Venus markets#53
Merged
test(contracts): CharonLiquidator fork-test suite across 5 Venus markets#53
Conversation
New `contracts/test/CharonLiquidatorFork.t.sol` exercises the
liquidator against a BNB Smart Chain mainnet fork. The real value
these tests add over the pre-existing unit-mock suite is the
integration with the live Aave V3 Pool — the only environment where
`Pool.flashLoanSimple` (proxy → implementation → aToken.transfer →
`executeOperation` → pull-back via approval) runs unmodified.
Suite shape:
- A. Parametric happy-path (`test_forkHappyPath_acrossAllMarkets`)
iterates five (debt, collateral) pairs spanning stable/stable,
stable/volatile, volatile/stable, and mixed-decimal combinations.
Each iteration deploys a clean liquidator, seeds the Aave premium
plus a small profit buffer, mocks Venus (liquidateBorrow,
balanceOf, redeem) and PancakeSwap (exactInputSingle), and asserts
the owner received profit and that `LiquidationExecuted` fired
with matching topics. Event matching uses `vm.recordLogs` + topic
walk rather than `vm.expectEmit` — more resilient inside a loop
where non-target logs fire between setup and the emit of interest.
- B. Slippage edge cases:
- `test_fork_slippage_tooTight_reverts` mocks the router to revert
with PancakeSwap's canonical `"Too little received"` string and
expects that revert to propagate.
- `test_fork_underRepayment_reverts` seeds zero surplus so the
post-swap balance falls short of `amount + premium` and the
contract's `"swap output below repayment"` defensive guard
fires.
- C. `test_fork_realContractsHaveCode` is a front-door sanity check
that surfaces a clear failure message if the configured BSC RPC
endpoint doesn't expose the pinned mainnet addresses.
Notable exclusions:
- BUSD is deliberately absent. Its Aave V3 reserve on BSC has been
deactivated; `flashLoanSimple(BUSD, …)` now reverts with
`ReserveInactive()` regardless of the contract under test.
- vBNB is excluded because its redemption yields native BNB, which
`CharonLiquidator` assumes is ERC-20 — a separate WBNB-wrap path
is needed before the contract can serve that market.
Mock strategy (Venus + PCS) trades realism for determinism:
identifying an underwater borrower + a stable PCS V3 pair at a
pinned block would need ongoing archive research that doesn't
belong in a regression suite. The previously-skipped
`test_executeLiquidation_endToEndOnFork` is left in place as the
marker for that future unmocked-PCS work.
Closes #23.
This was referenced Apr 22, 2026
[contracts] batchExecute missing from fork test suite — batch path has no Aave V3 fork coverage
#268
Closed
Closed
obchain
added a commit
that referenced
this pull request
Apr 23, 2026
add in-file Venus vToken, pcs v3 swap router, and ERC-20 mocks so executeOperation can be driven end-to-end without a mainnet fork. new coverage: - full happy-path callback: liquidateBorrow + redeem + swap + profit sweep, with vm.expectEmit on LiquidationExecuted asserting exact profit value and indexed topics. - asset/debt and amount/repay mismatch guards both revert. - two slippage paths: router-side `amountOutMinimum` enforcement with a nonzero minSwapOut, plus the defensive `swap output below repayment` check when minSwapOut is zero. - constructor zero-address guards for aavePool and swapRouter. - rescue() to a contract recipient whose receive() reverts — pins the 2300-gas `transfer` behaviour (issue #135) and documents the loss-of-funds safety of the current path. extend MockERC20 with allowance-aware `approve` and `transferFrom` (needed by the mock router's real `transferFrom` pull). retarget the skipped fork test to issue #53 (feat/25) — the placeholder pointed at a non-existent #22.x.
This was referenced Apr 23, 2026
Closed
solc 0.8.24 defaults to Shanghai codegen and emits PUSH0 (0x5f). BSC mainnet runs a pre-Shanghai EVM, so every deploy of the fork suite's CharonLiquidator lands bytecode that would fault on BSC. forge-test runs against revm which accepts PUSH0, so every passing fork test was a false signal against the wrong artifact. Add `evm_version = "paris"` to contracts/foundry.toml and strip the caret from every `pragma solidity ^0.8.24;` so a future solc patch cannot silently shift codegen under the same source tree. Fork tests now exercise the same bytecode shape that would deploy to BSC mainnet. Closes #263
setUp() called vm.createSelectFork("bnb") with no block, so every
CI run forked whatever head the archive RPC exposed. Aave V3
reserve state, vToken exchange rates, and ERC-20 balances drift
block-to-block — a green run one day was not a guarantee of a
green run the next, and a broken fix could slip through on a
favourable fork height.
Pin FORK_BLOCK = 94_000_000 (captured 2026-04-23; every referenced
Aave V3 reserve and Venus vToken is live at that height). Keep an
escape hatch via `vm.envOr("BSC_FORK_BLOCK", FORK_BLOCK)` so ad-hoc
investigation against a different block does not require editing
the source. Bump the constant in a dedicated, reviewed commit when
refreshing the baseline.
Closes #264
split the single five-market loop into five named per-market tests so a regression in one pool no longer masks the other four (#272). drop the inline keccak selector comparison and use the compiler- checked `CharonLiquidator.LiquidationExecuted` / `.BatchExecuted` event emitters; renaming the event now breaks the tests at compile time rather than at runtime (#273). stop mocking the pancakeswap v3 router on the happy path. the real on-fork router executes the collateral -> debt swap so the `amountOutMinimum` slippage floor, post-swap balance check and router-side pool accounting are all exercised end-to-end (#266). size `minSwapOut` from the real pancakeswap quoter v2 with a 50bps floor; add a dedicated slippage-revert test that sets `minSwapOut` one wei above the live quote and expects the router's canonical `"Too little received"` revert (#267). add `test_forkBatchExecute_uniqueCollateralMarkets_happyPath` to drive four markets with distinct collateral tokens through `CharonLiquidator.batchExecute` in one transaction. asserts one `LiquidationExecuted` per iteration, a single terminal `BatchExecuted(n)` with matching count, zero dust across every touched erc-20, and the full batch under the batch gas ceiling (#268). markets that share collateral underlyings (usdt/btcb and usdc/btcb) are deliberately excluded so the first iteration's real swap does not drain the shared seeded balance before the second iteration runs. snapshot gas around every liquidation call, log `gas_used_liquidation` / `gas_used_batch_unique`, and assert against per-market and per-batch ceilings sized at ~1.25x observed usage. fail fast on regressions (#274). probe for a real bsc archive rpc in `setUp` by reading multicall3 bytecode at `FORK_BLOCK - 5000`. if the endpoint is non-archive, `vm.skip` the entire suite with an operator-facing reason string so ci logs never misattribute a non-archive failure to contract logic (#269). scope note: #265 (cold-wallet profit sweep) and #270 (vbnb -> wbnb wrap) depend on contract changes that have not yet landed on this branch and are left for a follow-up commit once those contract changes arrive.
obchain
added a commit
that referenced
this pull request
Apr 23, 2026
Wraps Batcher::encode_calldata's output in an opaque UnsimulatedBatchCalldata newtype. The only promotion path is Batcher::simulate, which runs the buffer through Simulator::simulate and returns SimulatedBatchCalldata on success. A broadcaster written against this crate that accepts only SimulatedBatchCalldata cannot be handed raw encoder output by mistake, so the CLAUDE.md invariant "no broadcast without a passing eth_call" becomes a compile-time guarantee for the batch path (mirroring the UnverifiedPreSigned guard on the mempool pre-sign path). BatcherError gains a SimulationFailed variant carrying the node's revert string. Foundry-side batchExecute fork test stays on feat/25 / PR #53. Closes #298
CharonLiquidator gains an immutable `coldWallet` constructor arg.
Profit is swept to coldWallet instead of owner on every successful
executeOperation (and therefore every executeLiquidation and every
iteration of batchExecute). Enforces the CLAUDE.md safety invariant
("profit must not park at the hot wallet") at the contract layer so
a compromised bot key cannot drain accumulated earnings.
Constructor requires coldWallet != address(0) and
coldWallet != msg.sender — immutability makes the check one-shot
so the runtime cost is zero per liquidation.
Fork test now deploys every liquidator with a distinct coldWallet
(makeAddr) and the happy-path assertion checks BOTH that the cold
wallet received profit AND that the hot wallet balance did NOT
grow. The double-sided check catches any future contract change
that would split profit between wallets (a single-sided assert
would silently pass that regression).
Also adds a fail-loud `vm.skip(true)` placeholder for vBNB native
collateral fork coverage so #121's eventual fix lands on a visible
scaffold instead of yet another missing-regression-guard gap. Body
documents the wrap-to-WBNB plus cold-wallet profit-sweep assertion
expected of the real test.
Unit suite green (20 pass), fork test setUp correctly errors on an
unset BNB_HTTP_URL when run without a live archive RPC.
Closes #265
Closes #270
…tests # Conflicts: # contracts/foundry.toml # contracts/src/CharonLiquidator.sol # contracts/test/CharonLiquidator.t.sol
- Add `swapPoolFee: 3000` to `_params` so the on-fork swap tier matches the tier quoted via Quoter V2 in `_minOutFromQuoter`. Without it, the post-#38 `LiquidationParams` struct literal is missing a field and `_validate`'s `swapPoolFee > 0` check rejects the call. - Expect the post-#38 `LiquidationExecuted` event shape (3 indexed topics including `recipient`, 2 data fields). Asserting the `recipient` topic is load-bearing: it pins the CLAUDE.md cold-wallet invariant at the log level, mirroring the runtime balance assertions. - Gate `setUp` on `BNB_HTTP_URL` before calling `vm.createSelectFork`. The `bnb` RPC alias resolves the env var at fork-create time and raises a hard failure when missing; the gate mirrors the skip-on-env pattern used by the unit suite so CI without the env var skips cleanly.
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
1 participant
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.
Summary
contracts/test/CharonLiquidatorFork.t.sol: 4 tests, 5-market parametric happy path + 2 slippage edge cases + 1 fork sanity checkBNB_HTTP_URLaliased asbnbincontracts/foundry.toml)Why fork tests at all
Pool.flashLoanSimplewalks proxy → implementation → aToken.transfer →executeOperation→ pull-back. That sequence is the single piece ofCharonLiquidatorthat can't be meaningfully tested against inline mocks — if Aave changes its callback wiring we need the fork to catch it. Unit tests inCharonLiquidator.t.solstay authoritative for access control, input validation, rescue(), and reentrancy.Markets covered
BUSD is intentionally excluded — Aave V3 BSC has deactivated the BUSD reserve, so
flashLoanSimple(BUSD, …)reverts withReserveInactive()and the error is not related to contract logic. vBNB is excluded because its redemption yields native BNB, which the contract treats as ERC-20 (a wrap-path follow-up).Slippage edges
"Too little received"→ the revert must propagate"swap output below repayment"guard firesTest plan
forge buildclean (same pre-existing 2 lint warnings onCharonLiquidator.sol, unchanged)forge test— 24 pass + 1 skip (pre-existing PCS research placeholder)https://bsc-rpc.publicnode.comtest_executeLiquidation_endToEndOnForkonce a stable PCS V3 pair + block is identified (outside M4 scope)Stacked PR
Base is
feat/24-anvil-fork(PR #52). Merge order: #46 → #50 → #51 → #52 → this.Closes #23.