-
Notifications
You must be signed in to change notification settings - Fork 6
V5 abis and order types #399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughIntroduces versioned ABI namespaces for Orderbook (V4, V5), adds deployer eval4 ABI and related structs, migrates core logic/tests to use ABI.Orderbook.V4 paths, adds versioned Order types (V3, V4) with decoding utilities, adjusts vaultId handling to BigInt in places, and standardizes certain wallet formatting to 18 decimals. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Core as Core Logic
participant ABI as OrderbookAbi (V4/V5)
participant Viem as viem encoder/decoder
Caller->>Core: request to build calldata (e.g., arb3 / takeOrders2)
Core->>ABI: select ABI.Orderbook.V4.Primary.*
Core->>Viem: encodeFunctionData(abi=V4.*, fn, args)
Viem-->>Core: encoded calldata
Core-->>Caller: transaction data
note over ABI,Core: ABI selection moved from Primary.* to V4.Primary.*
sequenceDiagram
autonumber
actor Consumer
participant OrderV3 as Order.V3
participant ABI as ABI.Orderbook.V4.Primary.OrderStructAbi
participant Viem as viem.decodeAbiParameters
Consumer->>OrderV3: tryFromBytes(orderBytes)
OrderV3->>Viem: decodeAbiParameters(ABI, orderBytes)
alt success
Viem-->>OrderV3: decoded fields
OrderV3-->>Consumer: Result.ok({ type: V3, owner, nonce, evaluable, validInputs/Outputs })
else failure
Viem--x OrderV3: error
OrderV3-->>Consumer: Result.err(error)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (24)
Comment |
Motivation
Do NOT merge before #397
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit