chore: add multi-bridge support to order book markets#1302
Conversation
Time Submission Status
You can submit time with the command. Example: See available commands to help comply with our Guidelines. |
📝 WalkthroughWalkthroughAdds bridge support to the order-book: a new non-null Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Client
participant CreateMarket as create_market Action
participant OBQueries as ob_queries Table
participant Matching as Matching Engine
participant Vault as Vault Ops
participant Bridge as Bridge Dispatcher
Test->>CreateMarket: create_market($bridge, $query_hash, ...)
CreateMarket->>OBQueries: INSERT ... bridge=$bridge
OBQueries-->>CreateMarket: query_id
Note over Test,Matching: Order lifecycle / matching
Test->>Matching: match_orders($query_id, ..., $bridge)
Matching->>OBQueries: SELECT bridge FROM ob_queries WHERE query_id
OBQueries-->>Matching: bridge
Matching->>Vault: ob_lock_collateral($bridge, $amount)
Vault->>Bridge: route request (CASE $bridge)
Bridge->>Bridge: call hoodi_tt2 / sepolia_bridge / ethereum_bridge
Bridge-->>Vault: result
Vault-->>Matching: success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
internal/migrations/032-order-book-actions.sql (3)
561-561: Critical: Recursive call missing$bridgeparameter.The recursive call to
match_directis missing the$bridgeparameter. The function signature requires 4 parameters($query_id INT, $outcome BOOL, $price INT, $bridge TEXT), but only 3 are passed.This will cause the recursive matching to fail.
🐛 Proposed fix
-- Recursively call to match next orders - match_direct($query_id, $outcome, $price); + match_direct($query_id, $outcome, $price, $bridge); };
711-711: Critical: Recursive call missing$bridgeparameter.Same issue as
match_direct- the recursive call tomatch_mintis missing the required$bridgeparameter.🐛 Proposed fix
-- Recursively match next orders - match_mint($query_id, $yes_price, $no_price); + match_mint($query_id, $yes_price, $no_price, $bridge); };
879-879: Critical: Recursive call missing$bridgeparameter.Same issue - the recursive call to
match_burnis missing the required$bridgeparameter.🐛 Proposed fix
-- Recursively match next orders - match_burn($query_id, $yes_price, $no_price); + match_burn($query_id, $yes_price, $no_price, $bridge); };
🤖 Fix all issues with AI agents
In `@internal/migrations/031-order-book-vault.sql`:
- Around line 29-50: The comment in ob_lock_collateral claims validate_bridge is
called but no such call exists and the validate_bridge procedure is defined in a
later migration (032), so either (A) remove or update the misleading comment to
state that bridge validation is performed inline via the final else branch
(supported bridges: hoodi_tt2, sepolia_bridge, ethereum_bridge), or (B) if you
intend to call validate_bridge from ob_lock_collateral, move or ensure
validate_bridge is available before this migration and add a call to
validate_bridge($bridge) at the start of ob_lock_collateral; reference:
ob_lock_collateral and validate_bridge.
- Around line 17-22: The comment in the migration references a procedure named
validate_bridge that does not exist yet at runtime (it’s defined in a later
migration), so update the comment in 031-order-book-vault.sql to avoid implying
the procedure is available: either remove the reference to validate_bridge
entirely or reword to explicitly state that validate_bridge is defined in a
subsequent migration (migration 032) and is used by this migration, so readers
aren’t misled about runtime availability; ensure the updated comment mentions
the procedure name validate_bridge so it’s easy to locate.
🧹 Nitpick comments (4)
internal/migrations/030-order-book-schema.sql (1)
31-31: Consider adding a CHECK constraint for valid bridge values.The
bridgecolumn is added without validation constraints. Given that031-order-book-vault.sqlexplicitly validates bridges against a known list (hoodi_tt2,sepolia_bridge,ethereum_bridge), consider adding a CHECK constraint here for data integrity at the schema level.♻️ Suggested constraint
bridge TEXT NOT NULL, + CONSTRAINT chk_ob_queries_bridge CHECK (bridge IN ('hoodi_tt2', 'sepolia_bridge', 'ethereum_bridge')),tests/streams/order_book/queries_test.go (1)
27-31: Unused constanttestExtensionNameQueriesdefined.This file defines
testExtensionNameQueriesbut Line 880 usestestExtensionName(frommarket_creation_test.go). Since both are in the same package with identical values, this works but creates confusion.♻️ Suggested fix - use the local constant
func createTestMarketQueries(t *testing.T, ctx context.Context, platform *kwilTesting.Platform, signer *util.EthereumAddress) (int, []byte) { queryData := []byte(fmt.Sprintf("test:stream:%d", time.Now().UnixNano())) queryHash := sha256.Sum256(queryData) settleTime := time.Now().Add(1 * time.Hour).Unix() var queryID int64 err := callActionQueries(ctx, platform, signer, "create_market", - []any{testExtensionName, queryHash[:], settleTime, int64(5), int64(20)}, + []any{testExtensionNameQueries, queryHash[:], settleTime, int64(5), int64(20)}, func(row *common.Row) error { queryID = row.Values[0].(int64) return nil }) require.NoError(t, err)internal/migrations/031-order-book-vault.sql (1)
41-49: Consider extracting bridge dispatch logic to reduce duplication.The bridge dispatch logic (if-else chain for
hoodi_tt2,sepolia_bridge,ethereum_bridge) is duplicated betweenob_lock_collateralandob_unlock_collateral. If more bridges are added, both functions must be updated.This is acceptable for now with 3 bridges, but consider centralizing the supported bridges list if the number grows.
internal/migrations/032-order-book-actions.sql (1)
135-162: Consider extracting bridge dispatch logic to reduce code duplication.The if-else chain for bridge-specific operations (
balance,lock,transfer,unlock) is repeated multiple times throughout this file. While currently functional, this pattern makes adding new bridges error-prone and increases maintenance burden.Consider creating helper actions like
ob_get_balance($bridge TEXT, $address TEXT),ob_lock($bridge TEXT, $amount NUMERIC), andob_transfer($bridge TEXT, $to TEXT, $amount NUMERIC)to centralize the dispatch logic.
1 participant
resolves: https://github.com/truflation/website/issues/2979
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.