14 KiB
Code Review: Gaps and Placeholders
Critical Gaps
1. Chain Registry - Hardcoded/Incorrect Addresses
Location: src/config/chains.ts
Issues:
- Line 70: Aave PoolDataProvider address is placeholder:
0x7B4C56Bf2616e8E2b5b2E5C5C5C5C5C5C5C5C5C5 - Line 82: Maker Jug address is placeholder:
0x19c0976f590D67707E62397C1B5Df5C4b3B3b3b3 - Line 83: Maker DaiJoin address is placeholder:
0x9759A6Ac90977b93B585a2242A5C5C5C5C5C5C5C5 - Line 102: USDT Chainlink oracle is placeholder:
0x3E7d1eAB1ad2CE9715bccD9772aF5C5C5C5C5C5C5 - Line 179: Base Aave PoolDataProvider is placeholder:
0x2d09890EF08c270b34F8A3D3C5C5C5C5C5C5C5C5 - Missing: Many protocol addresses for L2s (Arbitrum, Optimism, Base) are incomplete
- Missing: Chainlink oracle addresses for L2s are not configured
Impact: High - Will cause runtime failures when accessing these contracts
2. AtomicExecutor.sol - Flash Loan Callback Security Issue
Location: contracts/AtomicExecutor.sol:128
Issue:
require(msg.sender == initiator || msg.sender == address(this), "Unauthorized");
- The check
msg.sender == address(this)is incorrect - flash loan callback should only accept calls from the Aave Pool - Should verify
msg.senderis the Aave Pool address, notaddress(this)
Impact: Critical - Security vulnerability, could allow unauthorized flash loan callbacks
3. MakerDAO Adapter - Missing CDP ID Parsing
Location: src/adapters/maker.ts:80
Issue:
return 0n; // Placeholder
openVault()always returns0ninstead of parsing the actual CDP ID from transaction events- Comment says "In production, parse from Vat.cdp events" but not implemented
Impact: High - Cannot use returned CDP ID for subsequent operations
4. Aggregator Adapter - No Real API Integration
Location: src/adapters/aggregators.ts:59-67
Issue:
// In production, call 1inch API for off-chain quote
// For now, return placeholder
const minReturn = (amountIn * BigInt(10000 - slippageBps)) / 10000n;
return {
amountOut: minReturn, // Placeholder
data: "0x", // Would be encoded swap data from 1inch API
gasEstimate: 200000n,
};
- No actual 1inch API integration
- Returns fake quotes that don't reflect real market prices
- No swap data encoding
Impact: High - Cannot use aggregators for real swaps
5. Cross-Chain Orchestrator - Complete Placeholder
Location: src/xchain/orchestrator.ts
Issues:
executeCrossChain()returns hardcoded{ messageId: "0x", status: "pending" }checkMessageStatus()always returns"pending"executeCompensatingLeg()is empty- No CCIP, LayerZero, or Wormhole integration
Impact: High - Cross-chain functionality is non-functional
6. Cross-Chain Guards - Placeholder Implementation
Location: src/xchain/guards.ts:14
Issue:
// Placeholder for cross-chain guard evaluation
return {
passed: true,
status: "delivered",
};
- Always returns
passed: truewithout any actual checks - No finality threshold validation
- No message status polling
Impact: Medium - Cross-chain safety checks are bypassed
7. KMS/HSM Secret Store - Not Implemented
Location: src/utils/secrets.ts:31-40
Issue:
// TODO: Implement KMS/HSM/Safe module integration
export class KMSSecretStore implements SecretStore {
// Placeholder for KMS integration
async get(name: string): Promise<string | null> {
throw new Error("KMS integration not implemented");
}
- All methods throw "not implemented" errors
- No AWS KMS, HSM, or Safe module integration
Impact: Medium - Cannot use secure secret storage in production
8. CLI Template System - Not Implemented
Location: src/cli.ts:76
Issue:
// TODO: Implement template system
console.log("Template system coming soon");
strategic build --templatecommand does nothing
Impact: Low - Feature not available
Implementation Gaps
9. Compiler - Missing Action Types
Location: src/planner/compiler.ts
Missing Implementations:
aaveV3.flashLoan- Detected but not compiled into callsaaveV3.setUserEMode- Not in compileraaveV3.setUserUseReserveAsCollateral- Not in compilercompoundV3.withdraw- Not in compilercompoundV3.borrow- Not in compilercompoundV3.repay- Not in compilermaker.*actions - Not in compilerbalancer.*actions - Not in compilercurve.*actions - Not in compilerlido.*actions - Not in compilerpermit2.*actions - Not in compileraggregators.*actions - Not in compilerperps.*actions - Not in compiler
Impact: High - Most strategy actions cannot be executed
10. Flash Loan Integration - Incomplete
Location: src/planner/compiler.ts:67-70
Issue:
// If flash loan, wrap calls in flash loan callback
if (requiresFlashLoan && flashLoanAsset && flashLoanAmount) {
// Flash loan calls will be executed inside the callback
// The executor contract will handle this
}
- No actual wrapping logic
- Calls are not reorganized to execute inside flash loan callback
- No integration with
executeFlashLoan()in executor
Impact: High - Flash loan strategies won't work
11. Gas Estimation - Crude Approximation
Location: src/planner/compiler.ts:233-236
Issue:
private estimateGas(calls: CompiledCall[]): bigint {
// Rough estimate: 100k per call + 21k base
return BigInt(calls.length * 100000 + 21000);
}
- No actual gas estimation via
eth_estimateGas - Fixed 100k per call is inaccurate
- Doesn't account for different call complexities
Impact: Medium - Gas estimates may be wildly inaccurate
12. Fork Simulation - Basic Implementation
Location: src/engine.ts:185-213 and scripts/simulate.ts
Issues:
- Uses
anvil_resetwhich may not work with all RPC providers - No actual state snapshot/restore
- No calldata trace/debugging
- No revert diff analysis
- Simulation just calls
provider.call()without proper setup
Impact: Medium - Fork simulation is unreliable
13. Uniswap V3 Compiler - Hardcoded Recipient
Location: src/planner/compiler.ts:195
Issue:
recipient: "0x0000000000000000000000000000000000000000", // Will be set by executor
- Comment says "Will be set by executor" but executor doesn't modify calldata
- Should use actual executor address or strategy-defined recipient
Impact: High - Swaps may fail or send tokens to zero address
14. Price Oracle - Hardcoded Decimals
Location: src/pricing/index.ts:90
Issue:
decimals: 18, // Assume 18 decimals for now
- TWAP price assumes 18 decimals for all tokens
- Should fetch actual token decimals
Impact: Medium - Price calculations may be incorrect for non-18-decimal tokens
15. Price Oracle - Weighted Average Bug
Location: src/pricing/index.ts:146-155
Issue:
let weightedSum = 0n;
let totalWeight = 0;
for (const source of sources) {
const weight = source.name === "chainlink" ? 0.7 : 0.3;
weightedSum += (source.price * BigInt(Math.floor(weight * 1000))) / 1000n;
totalWeight += weight;
}
const price = totalWeight > 0 ? weightedSum / BigInt(Math.floor(totalWeight * 1000)) * 1000n : sources[0].price;
- Division logic is incorrect - divides by
totalWeight * 1000then multiplies by 1000 - Should divide by
totalWeightdirectly - Weighted average calculation is mathematically wrong
Impact: High - Price calculations are incorrect
16. Permit2 - Not Integrated in Compiler
Location: src/utils/permit.ts exists but src/planner/compiler.ts doesn't use it
Issue:
- Permit2 signing functions exist but are never called
- Compiler doesn't check for
needsApproval()before operations - No automatic permit generation in strategy execution
Impact: Medium - Cannot use Permit2 to avoid approvals
17. Flashbots Bundle - Missing Integration
Location: src/wallets/bundles.ts exists but src/engine.ts doesn't use it
Issue:
- Flashbots bundle manager exists but execution engine doesn't integrate it
- No option to submit via Flashbots in CLI
- No bundle simulation before execution
Impact: Medium - Cannot use Flashbots for MEV protection
18. Telemetry - Simple Hash Implementation
Location: src/telemetry.ts:35-38
Issue:
export function getStrategyHash(strategy: any): string {
// Simple hash of strategy JSON
const json = JSON.stringify(strategy);
// In production, use crypto.createHash
return Buffer.from(json).toString("base64").slice(0, 16);
}
- Comment says "In production, use crypto.createHash" but uses base64 encoding
- Not a cryptographic hash, just base64 encoding
Impact: Low - Hash is not cryptographically secure but functional
19. Aave V3 Adapter - Missing Error Handling
Location: src/adapters/aaveV3.ts
Issues:
- No validation of asset addresses
- No check if asset is supported by Aave
- No handling of paused reserves
withdraw()doesn't parse actual withdrawal amount from events (line 91 comment)
Impact: Medium - May fail silently or with unclear errors
20. Strategy Schema - Missing Action Types
Location: src/strategy.schema.ts
Missing from schema but adapters exist:
maker.openVault,maker.frob,maker.join,maker.exitbalancer.swap,balancer.batchSwapcurve.exchange,curve.exchange_underlyinglido.wrap,lido.unwrappermit2.permitaggregators.swap1Inch,aggregators.swapZeroExperps.increasePosition,perps.decreasePosition
Impact: High - Cannot define strategies using these actions
21. Executor Contract - Missing Flash Loan Interface
Location: contracts/AtomicExecutor.sol:8-16
Issue:
- Defines
IPoolinterface locally but Aave v3 usesIFlashLoanSimpleReceiver - Missing proper interface implementation
- Should import or define the full receiver interface
Impact: Medium - May not properly implement Aave's callback interface
22. Executor Tests - Incomplete
Location: contracts/test/AtomicExecutor.t.sol
Issues:
- Test target contract doesn't exist (calls
target.test()) - No actual flash loan test
- No test for flash loan callback
- Tests are minimal and don't cover edge cases
Impact: Medium - Contract not properly tested
23. Deploy Script - Hardcoded Addresses
Location: scripts/Deploy.s.sol
Issue:
- Hardcodes protocol addresses that may not exist on all chains
- No chain-specific configuration
- Doesn't verify addresses before allowing
Impact: Medium - Deployment may fail on different chains
24. Example Strategies - Invalid References
Location: strategies/sample.recursive.json and others
Issues:
- Uses
{{executor}}placeholder in guards but no substitution logic - Uses token addresses that may not exist
- No validation that strategies are actually executable
Impact: Low - Examples may not work out of the box
Data/Configuration Gaps
25. Missing Protocol Addresses
Missing for L2s:
- MakerDAO addresses (only mainnet)
- Curve registry (only mainnet)
- Lido (incomplete for L2s)
- Aggregators (only mainnet)
- Chainlink oracles (incomplete)
Impact: High - Cannot use these protocols on L2s
26. Missing ABIs
Location: All adapter files use "simplified" ABIs
Issues:
- ABIs are minimal and may be missing required functions
- No full contract ABIs imported
- May miss important events or return values
Impact: Medium - Some operations may fail or miss data
27. Risk Config - Static Defaults
Location: src/config/risk.ts
Issue:
- Always returns
DEFAULT_RISK_CONFIG - No per-chain configuration
- No loading from file/env
- No dynamic risk adjustment
Impact: Low - Risk settings are not customizable
Testing Gaps
28. No Unit Tests
Location: tests/unit/ directory is empty
Impact: High - No test coverage for TypeScript code
29. No Integration Tests
Location: tests/integration/ directory is empty
Impact: High - No end-to-end testing
30. Foundry Tests - Minimal
Location: contracts/test/AtomicExecutor.t.sol
Impact: Medium - Contract has basic tests only
Documentation Gaps
31. Missing API Documentation
- No JSDoc comments on public methods
- No usage examples for adapters
- No guard parameter documentation
Impact: Low - Harder for developers to use
32. Missing Architecture Documentation
- No diagrams of execution flow
- No explanation of flash loan callback mechanism
- No guard evaluation order documentation
Impact: Low - Harder to understand system
Summary
Critical Issues (Must Fix):
- AtomicExecutor flash loan callback security (item #2)
- Chain registry placeholder addresses (item #1)
- Compiler missing action types (item #9)
- Flash loan integration incomplete (item #10)
- Price oracle weighted average bug (item #15)
High Priority (Should Fix): 6. MakerDAO CDP ID parsing (item #3) 7. Aggregator API integration (item #4) 8. Uniswap recipient address (item #13) 9. Missing action types in schema (item #20) 10. Missing protocol addresses for L2s (item #25)
Medium Priority (Nice to Have): 11. Cross-chain orchestrator (item #5) 12. Gas estimation accuracy (item #11) 13. Fork simulation improvements (item #12) 14. Permit2 integration (item #16) 15. Flashbots integration (item #17)
Low Priority (Future Work): 16. KMS/HSM integration (item #7) 17. Template system (item #8) 18. Testing coverage (items #28-30) 19. Documentation (items #31-32)