feat: register generic ERC4626 validators with embedded vault registry#9
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/validators/evm/erc4626/vault-config.ts (1)
24-33: Address normalization looks correct.The mapping correctly lowercases
address,inputTokenAddress, andvaultTokenAddress. However, ifcanEnter/canExitare present in the registry, consider adding them to the mapping:♻️ Proposed fix to include canEnter/canExit
interface RegistryEntry { yieldId: string; address: string; chainId: number; protocol: string; network: string; inputTokenAddress: string; vaultTokenAddress: string; isWethVault: boolean; + canEnter?: boolean; + canExit?: boolean; } // ... const vaults: VaultInfo [] = registry.vaults.map((entry) => ({ address: entry.address.toLowerCase(), chainId: entry.chainId, protocol: entry.protocol, yieldId: entry.yieldId, inputTokenAddress: entry.inputTokenAddress.toLowerCase(), vaultTokenAddress: entry.vaultTokenAddress.toLowerCase(), network: entry.network, isWethVault: entry.isWethVault, + canEnter: entry.canEnter, + canExit: entry.canExit, }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/validators/evm/erc4626/vault-config.ts` around lines 24 - 33, The vault mapping currently builds VaultInfo objects from registry.vaults but omits optional canEnter/canExit flags; update the mapping in the vaults creation (the registry.vaults.map lambda that produces address, chainId, protocol, yieldId, inputTokenAddress, vaultTokenAddress, network, isWethVault) to also copy over canEnter and canExit when present (e.g., add canEnter: entry.canEnter and canExit: entry.canExit), preserving existing lowercasing behavior for addresses and leaving fields undefined or their registry values if the flags are absent.src/validators/evm/erc4626/erc4626.exhaustive-coverage.test.ts (1)
20-31: Consider exporting WETH addresses from validator to reduce duplication.This
WETH_ADDRESSESmap duplicates the WETH address configuration that exists inerc4626.validator.ts. If these fall out of sync, tests may pass but production validation could fail (or vice versa).♻️ Suggested approach to avoid duplication
Export the WETH addresses map from the validator module:
// In erc4626.validator.ts export const WETH_ADDRESSES: Record<number, string> = { ... };Then import in test:
import { WETH_ADDRESSES } from './erc4626.validator';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/validators/evm/erc4626/erc4626.exhaustive-coverage.test.ts` around lines 20 - 31, Replace the duplicate WETH_ADDRESSES map in the test with a single exported constant from the validator: export the existing WETH_ADDRESSES (type Record<number,string>) from the erc4626.validator module and update the test file to import { WETH_ADDRESSES } from that module instead of redefining it; ensure the exported name and type match the test usage so both test and production code reference the same map.src/validators/evm/erc4626/erc4626.protocol-coverage.test.ts (1)
21-32: Consider centralizing WETH address constants to prevent drift.This map duplicates the validator’s WETH address list; a shared constant would keep tests and runtime in sync.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/validators/evm/erc4626/erc4626.protocol-coverage.test.ts` around lines 21 - 32, The test-level WETH_ADDRESSES map duplicates the runtime validator's list; extract the canonical WETH_ADDRESSES constant into a single exported module (e.g., export const WETH_ADDRESSES) and have both the validator code and this test import that shared constant instead of redefining it; update references in the validator (the existing runtime constant/name) and in the test (current WETH_ADDRESSES) to import from the new shared module so tests and runtime stay in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/validators/evm/erc4626/erc4626.protocol-coverage.test.ts`:
- Around line 128-175: Run the project's Prettier/formatter over the "WETH vault
coverage" test block to fix indentation and spacing issues; locate the
describe('WETH vault coverage', ...) block (which contains the wethVault
constant, wethAddr, validator = new ERC4626Validator(...), and the WRAP/UNWRAP
it() cases) and reformat that entire section so it matches the repository's
Prettier rules (ensure consistent indentation, spacing around
parentheses/braces, and line breaks) before committing.
- Line 7: Replace the CommonJS require call with an ES6 import: change the
statement that imports GENERIC_ERC4626_PROTOCOLS (currently using
require('../../index')) to use an import declaration that imports the named
export GENERIC_ERC4626_PROTOCOLS from the module path; update any test usage
assuming the same exported symbol name so the test continues to reference
GENERIC_ERC4626_PROTOCOLS without altering its behavior.
In `@src/validators/evm/erc4626/erc4626.validator.ts`:
- Around line 48-50: The whitespace before "new" in the static WETH interface
declaration causes Prettier to flag formatting; update the declaration of
wethInterface so it matches the other static interface lines (use "private
static readonly wethInterface = new ethers.Interface(WETH_ABI);" with a single
space before the '=' and no extra space before "new")—locate the wethInterface
symbol in the erc4626.validator.ts file alongside erc4626Interface and
erc20Interface and normalize spacing to match those lines.
In `@src/validators/evm/erc4626/vault-config.ts`:
- Around line 4-13: RegistryEntry is missing the optional canEnter and canExit
fields so those values from the embedded registry JSON get dropped; update the
RegistryEntry interface to include optional boolean canEnter? and canExit? and
then ensure loadEmbeddedRegistry() (and any mapping to VaultInfo) preserves and
maps these fields into the resulting VaultInfo objects so optional values in the
JSON are not lost.
---
Nitpick comments:
In `@src/validators/evm/erc4626/erc4626.exhaustive-coverage.test.ts`:
- Around line 20-31: Replace the duplicate WETH_ADDRESSES map in the test with a
single exported constant from the validator: export the existing WETH_ADDRESSES
(type Record<number,string>) from the erc4626.validator module and update the
test file to import { WETH_ADDRESSES } from that module instead of redefining
it; ensure the exported name and type match the test usage so both test and
production code reference the same map.
In `@src/validators/evm/erc4626/erc4626.protocol-coverage.test.ts`:
- Around line 21-32: The test-level WETH_ADDRESSES map duplicates the runtime
validator's list; extract the canonical WETH_ADDRESSES constant into a single
exported module (e.g., export const WETH_ADDRESSES) and have both the validator
code and this test import that shared constant instead of redefining it; update
references in the validator (the existing runtime constant/name) and in the test
(current WETH_ADDRESSES) to import from the new shared module so tests and
runtime stay in sync.
In `@src/validators/evm/erc4626/vault-config.ts`:
- Around line 24-33: The vault mapping currently builds VaultInfo objects from
registry.vaults but omits optional canEnter/canExit flags; update the mapping in
the vaults creation (the registry.vaults.map lambda that produces address,
chainId, protocol, yieldId, inputTokenAddress, vaultTokenAddress, network,
isWethVault) to also copy over canEnter and canExit when present (e.g., add
canEnter: entry.canEnter and canExit: entry.canExit), preserving existing
lowercasing behavior for addresses and leaving fields undefined or their
registry values if the flags are absent.
raiseerco
left a comment
There was a problem hiding this comment.
minor readability comment, all lgtm
0887324
into
eng-1236-implement-generic-erc-4626-shield-adapter
Summary by CodeRabbit
Release Notes
New Features
Documentation
vault-registry.jsonfrom the monorepo's shield registry endpoint, removing any runtime API dependencyERC4626Validatorinstance for each vault belonging to an allowed protocol (12 protocols, ~1,400 vaults)GENERIC_ERC4626_PROTOCOLS) to gate registration — only protocols confirmed to use the standard ERC4626deposit/redeemflow are includedethers.Interfaceobjectsstatic readonlyon the validator class to share across all ~1,400 instances1.2.1Allowed protocols
Angle, Curve, Euler, Fluid, Gearbox, Idle Finance, Lista, Morpho, Sky, Summer.fi, Yearn, Yo Protocol
Aave, Maple, Spark, and Yield-xyz (OAV) ERC4626 vaults use non-standard transaction flows and are excluded until protocol-specific validators are added.
Files changed (this PR only — on top of PR #1)
src/validators/evm/erc4626/vault-registry.json/v1/shield/registrysrc/validators/evm/erc4626/vault-config.tsloadEmbeddedRegistry()loadersrc/validators/evm/erc4626/erc4626.validator.tsethers.Interfaceobjects, WETH gating invalidateWrap/validateUnwrapsrc/validators/index.tsGENERIC_ERC4626_PROTOCOLS), per-yield-ID registration looppackage.json1.2.1, addedvault-registry.jsontofilesREADME.mdTest coverage
erc4626.validator.test.tsvault-registry.test.tserc4626.protocol-coverage.test.tserc4626.exhaustive-coverage.test.tsHow to test
Design decisions
isSupported(yieldId)returns precise results and prevents cross-yield confusionethers.Interface— one ABI parse shared across ~1,400 instances instead of 4,200 redundant allocationsKnown gaps
gasLimit,maxFeePerGas,maxPriorityFeePerGas) are not validated — tracked separately as a shield-wide concern