feat: auto enter#57
Conversation
StructHash additions, V3Automation executeAutoEnter/executeFollowUp, EIP-712 bump to 6.0, 18 Foundry tests. See cross-repo contracts in §2.
…ollowUp
StructHash:
- New types: PoolSelection, AutoEnterAction, FollowUpTemplate, AutoEnterConfig
- Recompute OrderConfig_TYPEHASH and Order_TYPEHASH to include the new
autoEnterConfig field (the recursive type-hash propagates upward).
- New constants use keccak256("...") inline so the string and hash can't drift.
V3Automation (v6.0):
- Bump EIP-712 version from 5.0 to 6.0 (the OrderConfig type-hash change
invalidates all v5 signatures, so a new deployment is required).
- New Action.AUTO_ENTER + ExecuteAutoEnterParams/ExecuteFollowUpParams structs.
- executeAutoEnter pulls source tokens from the signer, deducts protocol+gas
fees, calls inherited _swapAndMint, transfers NFT to signer, persists
(orderDigest -> mintedTokenId, signer) for follow-up authorization.
- executeFollowUp re-uses the parent order's signature as authorization for
pre-templated rebalance/compound/exit/harvest against the new tokenId.
- Verifies followUpConfigEncoded matches the signed templateConfigHash.
Notes:
- Recipient = signer for the NFT. For follow-ups to work the user must
setApprovalForAll(V3Automation, true) before signing — same prerequisite
as the existing rebalance/compound flow.
- v1 rejects dynamic pool selection on-chain (PoolSelection.mode != 0).
The schema reserves space so Phase 2 is additive.
Validates the cross-repo invariant most likely to drift: the EIP-712 type-hash computation. Frontend signs, contract verifies — if these diverge, every auto-enter order fails verification silently. Covered: - DOMAIN_SEPARATOR matches v6.0 manually-computed value - FollowUpTemplate type-hash is stable - PoolSelection / AutoEnterAction hashes are deterministic and field-sensitive - Full Order: sign with vm.sign, recover via ECDSA, signer round-trips - Tampering with any field invalidates the recovered signer - Follow-ups participate in the signed digest Deferred to integration tests (need a fork): full mint, follow-up execution, revert paths for expired/wrong-signer/slippage/cancellation. Plan §4.5 lists the 18 integration scenarios.
V3Automation.executeAutoEnter: - Handle msg.value > 0 when sourceToken == WETH: deposit ETH to WETH and pull only the remainder from the signer. If msg.value is supplied but sourceToken != WETH, revert NoEtherToken (rather than silently leaving ETH stuck in the contract). - TooMuchEtherSent if msg.value > p.sourceAmount. script/UpgradeV6.s.sol: - Deploys V3Automation v6 with CREATE2 salt "V3AutomationV6" (distinct from v5's salt so addresses don't collide). Initializes + grants OPERATOR_ROLE to operators from env. Leaves v5 deployment in place for in-flight orders to drain.
The Solidity StructHash library only exposes _hash(bytes) externally — so abigen against the library's ABI doesn't generate Go struct types for Order/OrderConfig/AutoEnterConfig etc. StructHashEncoder is a pure-view wrapper that takes the relevant structs by calldata, exposing them in the contract's ABI. Backend's abi_gen.sh consumes StructHashEncoder.json -> libs/contracts/krystalvaultv6/ to get Go bindings with StructHashOrder, StructHashAutoEnterConfig, StructHashPoolSelection, StructHashFollowUpTemplate types. The wrapper itself is never deployed — it exists purely for ABI generation.
![krystal-reviewer[bot]](https://avatars.githubusercontent.com/in/3764494?s=60&v=4){kind=small}
PR Review SummaryReviewed the recent commits on Key findings:
I could not run Foundry tests in this environment because Verdict: Request Changes (do not merge until the authorization/replay and execution-param validation issues are fixed). |
…ount routing - executeAutoEnter now marks the parent order digest consumed before pulling funds (_executedAutoEnter), so a valid signature can't be replayed to mint repeatedly until allowance/balance is drained. - Bind execution to the signed order beyond the NFPM whitelist: require order.nfpmAddress == p.nfpm, ps.poolManagerOrNfpm == p.nfpm, ps.protocol == 0 (UNI_V3; v4/pancake belong on V4UtilsRouter), ps.hooks == 0, filterHash == 0. Prevents an operator redirecting the same signature to another whitelisted manager/protocol with matching pool params. - _buildAutoEnterSwapMintParams routes the funded amount into amount0/amount1 when the source token is a pool token (amount2 is only for third-token zaps); _swapAndPrepareAmounts ignores amount2 in the token0/token1 branches, so a direct token0/token1 auto-enter previously minted zero input or reverted. Tests: fork-free binding reverts (non-v3 protocol, non-zero hooks). NOT fixed here (require a cross-repo signed-schema change — the signed payload would need concrete min amounts / canonical execute params, touching StructHash + Go v6 signer + frontend hashing): enforcing signed swap/liquidity slippage bounds on-chain, and binding executeFollowUp's ExecuteParams (beyond action type) to the follow-up template.
PR Review SummaryReviewed the latest commit Status of prior issues:
New findings posted inline:
I could not run the test suite in this environment because Verdict: Request Changes. The recent fixes move this in the right direction, but the remaining authorization gaps around follow-up execution/slippage plus the protocol binding issue should be resolved before merge. |
…gured withdrawer - executeAutoEnter now requires p.protocol == Nfpm.Protocol.UNI_V3. _buildAutoEnterSwapMintParams forwards p.protocol into _swapAndMint (which picks the Nfpm.mint branch); forcing ps.protocol alone left the actual mint protocol caller-controlled. Test: rejects a mismatched p.protocol. - UpgradeV6 script grants WITHDRAWER_ROLE to the configured WITHDRAWER after initialize() (which only grants it to admin). Previously the env var was read but never used, so a separate withdrawer could never withdraw.
PR Review SummaryReviewed the latest commit Status of prior issues:
New finding posted inline:
I could not run the test suite in this environment because Verdict: Request Changes. The latest fixes address the recent protocol/withdrawer comments, but the remaining follow-up authorization gaps (including NFPM binding) and the acknowledged slippage/max-gas enforcement gap should be resolved before merge. |
…e amount executeFollowUp stored only the minted tokenId, so an operator could run a parent-authorized follow-up against an unrelated NFT of the same numeric id on a different whitelisted NFPM the signer also owns. Bind p.execute.nfpm/protocol to the parent's signed poolSelection (mirrors V4UtilsRouter's POSM binding). Also require p.sourceAmount == act.sourceAmount: the one-shot _executedAutoEnter guard permanently consumes the signature, and partial fills are not part of the signed schema, so a smaller pull would burn the order on a smaller-than-signed mint. Add test_AutoEnter_RejectsPartialAmount.
PR Review SummaryReviewed latest commit Status of prior issues:
No new inline comments were added because the remaining issues are already under discussion on existing inline comments. I could not run the test suite in this environment because Verdict: Request Changes. The latest commit fixes several important authorization gaps, but the remaining signed slippage/max-gas enforcement and follow-up parameter validation gaps should be resolved before merge. |
Adds pr-test.yaml running 'forge test --match-path test/AutoEnter.t.sol' on PRs to master and feat/auto-enter. The AUTO_ENTER suite is fork-free (no RPC/.env), so the gate is deterministic and green; previously v3utils had only the AI pi-reviewer workflow and no test gate on the feature branch.
PR Review SummaryReviewed the latest commit Status of prior issues:
New finding posted inline:
I could not run the test suite in this environment because Verdict: Request Changes. The CI addition is welcome, but it currently does not gate the intended branch, and the previously noted authorization gaps around signed slippage/max-gas and follow-up parameter validation should be resolved before merge. |
Parity with V4UtilsRouter's minLiquidity guard. Operator-supplied amountAddMin0/1 unbound to signed value; both zero disables V3Utils' mint slippage floor (dust mint). Require at least one non-zero (single-sided range legitimately has one zero side).
PR Review SummaryReviewed latest commit Status of prior issues:
No new inline comments were added because the remaining findings are already covered by existing review comments. I could not run the test suite in this environment because Verdict: Request Changes. The latest commit is a useful hardening step, but the remaining signed-constraint/follow-up authorization gaps and the CI branch filter should be fixed before merge. |
Add uint256 minLiquidity to the signed AutoEnterAction (parity with v4 StructHash) and enforce require(result.liquidity >= act.minLiquidity) post-mint in executeAutoEnter, so the operator cannot weaken the signed liquidity floor (replaces the amountAddMin>0 guard; operator amountAddMin0/1 remain as secondary per-token mins). Tests updated + new test_MinLiquidityAffectsOrderHash.
PR Review SummaryReviewed latest commit Status of prior issues:
No new inline comments were added because the remaining findings are already covered by existing review comments. I could not run the test suite in this environment because Verdict: Request Changes. The signed |
1 participant
No description provided.