chore: remove redundant decodeQueryComponent#1322
Merged
Merged
Conversation
Time Submission Status
You can submit time with the command. Example: See available commands to help comply with our Guidelines. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemoves query-components ABI decoding and its precompile/handler, deletes related tests and discovery action, drops denormalized ob_queries columns/indexes and associated market-return fields, and updates Go toolchain and multiple module dependencies. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (1)
internal/migrations/032-order-book-actions.sql (1)
239-266: Document thisget_market_inforeturn-shape change as a breaking API update.The SQL changes are consistent, but this alters the public action contract. Please add migration/release notes for client consumers expecting the old columns.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/migrations/032-order-book-actions.sql` around lines 239 - 266, The public action get_market_info had its return shape changed (column list now: hash, query_components, bridge, settle_time, settled, winning_outcome, settled_at, max_spread, min_order_size, created_at, creator); add a clear migration/release note documenting this breaking API change so clients know the new column order/names and any removed/renamed fields, include guidance for updating client queries that consume get_market_info (refer to get_market_info by name), the migration version or changelog entry referencing the 032-order-book-actions.sql commit, and an example of the old→new mapping and upgrade steps for consumers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 292: Remove the local module replace directive "replace
github.com/trufnetwork/sdk-go => ../sdk-go" from go.mod; locate the replace
entry and delete that line so the module uses the canonical remote module path,
and if local overrides are needed for development, create a go.work file instead
of committing this replace into go.mod.
- Line 71: The go.mod declares vulnerable deps; update the module versions: bump
github.com/ethereum/go-ethereum to at least v1.16.9 (or v1.17.0+ if you want all
fixes) and bump github.com/consensys/gnark-crypto to at least v0.18.1 (or
v0.19.2+ for a minor upgrade), then run module updates so imports in
extensions/tn_settlement, extensions/tn_utils, and extensions/tn_attestation
resolve against the patched releases; ensure go.mod/go.sum are updated (e.g.,
via go get/update and go mod tidy) and run tests/build to confirm no breakage.
---
Nitpick comments:
In `@internal/migrations/032-order-book-actions.sql`:
- Around line 239-266: The public action get_market_info had its return shape
changed (column list now: hash, query_components, bridge, settle_time, settled,
winning_outcome, settled_at, max_spread, min_order_size, created_at, creator);
add a clear migration/release note documenting this breaking API change so
clients know the new column order/names and any removed/renamed fields, include
guidance for updating client queries that consume get_market_info (refer to
get_market_info by name), the migration version or changelog entry referencing
the 032-order-book-actions.sql commit, and an example of the old→new mapping and
upgrade steps for consumers.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
extensions/tn_utils/datapoints.goextensions/tn_utils/precompiles.gogo.modinternal/migrations/030-order-book-schema.sqlinternal/migrations/032-order-book-actions.sqlinternal/migrations/043-get-markets-by-stream.sqltests/streams/order_book/discovery_test.go
💤 Files with no reviewable changes (4)
- tests/streams/order_book/discovery_test.go
- internal/migrations/030-order-book-schema.sql
- extensions/tn_utils/datapoints.go
- internal/migrations/043-get-markets-by-stream.sql
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yaml:
- Around line 29-35: The CI workflow currently uses golangci-lint-action@v8 with
version: latest which makes runs non-reproducible; update the golangci-lint job
(the job named "golangci-lint" and its input key "version") to pin a specific
linter release (for example replace latest with a concrete tag such as v2.10.1)
so CI uses a fixed linter version and only changes when you intentionally bump
it.
MicBun
force-pushed
the
moveDiscoverMarket
branch
from
a78a58b to
7e88d03
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/migrations/032-order-book-actions.sql (1)
239-251:⚠️ Potential issue | 🟠 MajorConfirmed breaking change:
get_market_infoPUBLIC VIEW return shape reduced from 15 to 11 fieldsThe migration removes four fields from the public action return tuple:
data_provider,stream_id,action_id, andquery_args. This breaks any external consumer expecting the old positional layout (e.g., offchain clients, SDKs, integrations decoding by position).Internal tests have been updated to the new 11-field structure. Ensure all external consumers are coordinated with this release, or deploy a versioned action variant (e.g.,
get_market_info_v1) to maintain backward compatibility during rollout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/migrations/032-order-book-actions.sql` around lines 239 - 251, The migration changes the PUBLIC VIEW action get_market_info to return 11 fields instead of the previous 15, removing data_provider, stream_id, action_id, and query_args which breaks consumers relying on positional layout; either restore the original 15-field return shape in get_market_info or add a backward-compatible action such as get_market_info_v1 that preserves the original return tuple (including data_provider, stream_id, action_id, query_args) and keep the new get_market_info for the updated shape so external SDKs/offchain clients can migrate without breaking changes.
♻️ Duplicate comments (1)
go.mod (1)
71-71:⚠️ Potential issue | 🔴 CriticalUpgrade vulnerable crypto/EVM dependencies before merge.
Line 71 and Line 94 still pin versions with known HIGH-severity advisories. Please bump:
github.com/consensys/gnark-cryptoto>= v0.18.1(or newer patched line),github.com/ethereum/go-ethereumto>= v1.17.0(covers the listed 2026 advisories).#!/bin/bash set -euo pipefail echo "Current pinned versions:" rg -n 'github.com/consensys/gnark-crypto|github.com/ethereum/go-ethereum' go.mod echo "" echo "Query OSV for pinned versions:" curl -s https://api.osv.dev/v1/query \ -H 'Content-Type: application/json' \ -d '{"package":{"ecosystem":"Go","name":"github.com/consensys/gnark-crypto"},"version":"0.18.0"}' | jq '.vulns[]?.id' curl -s https://api.osv.dev/v1/query \ -H 'Content-Type: application/json' \ -d '{"package":{"ecosystem":"Go","name":"github.com/ethereum/go-ethereum"},"version":"1.16.7"}' | jq '.vulns[]?.id'Also applies to: 94-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 71, The go.mod pins for github.com/consensys/gnark-crypto and github.com/ethereum/go-ethereum are vulnerable; update the dependency versions in go.mod so github.com/consensys/gnark-crypto is bumped to >= v0.18.1 and github.com/ethereum/go-ethereum is bumped to >= v1.17.0 (replace the current v0.18.0 and older go-ethereum entries), then run go get/go mod tidy to refresh the lockfile and verify no other modules require the older versions; target the two package names "github.com/consensys/gnark-crypto" and "github.com/ethereum/go-ethereum" when making the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@internal/migrations/032-order-book-actions.sql`:
- Around line 239-251: The migration changes the PUBLIC VIEW action
get_market_info to return 11 fields instead of the previous 15, removing
data_provider, stream_id, action_id, and query_args which breaks consumers
relying on positional layout; either restore the original 15-field return shape
in get_market_info or add a backward-compatible action such as
get_market_info_v1 that preserves the original return tuple (including
data_provider, stream_id, action_id, query_args) and keep the new
get_market_info for the updated shape so external SDKs/offchain clients can
migrate without breaking changes.
---
Duplicate comments:
In `@go.mod`:
- Line 71: The go.mod pins for github.com/consensys/gnark-crypto and
github.com/ethereum/go-ethereum are vulnerable; update the dependency versions
in go.mod so github.com/consensys/gnark-crypto is bumped to >= v0.18.1 and
github.com/ethereum/go-ethereum is bumped to >= v1.17.0 (replace the current
v0.18.0 and older go-ethereum entries), then run go get/go mod tidy to refresh
the lockfile and verify no other modules require the older versions; target the
two package names "github.com/consensys/gnark-crypto" and
"github.com/ethereum/go-ethereum" when making the changes.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
.golangci.yamlextensions/tn_utils/datapoints.goextensions/tn_utils/precompiles.goextensions/tn_utils/precompiles_decoding_test.gogo.modinternal/migrations/030-order-book-schema.sqlinternal/migrations/032-order-book-actions.sqlinternal/migrations/043-get-markets-by-stream.sqltests/streams/order_book/discovery_test.go
💤 Files with no reviewable changes (5)
- extensions/tn_utils/precompiles_decoding_test.go
- extensions/tn_utils/datapoints.go
- internal/migrations/030-order-book-schema.sql
- internal/migrations/043-get-markets-by-stream.sql
- tests/streams/order_book/discovery_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- extensions/tn_utils/precompiles.go
MicBun
force-pushed
the
moveDiscoverMarket
branch
from
c30813b to
aefe0a9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/tn_utils/precompiles.go`:
- Line 736: The current return uses []byte(sid) which incorrectly converts the
Go string representation to ASCII bytes; instead, ensure streamID is serialized
as the original bytes32 raw content before hashing in compute_attestation_hash:
either change DecodeQueryComponents to return streamID as []byte (preferred) or,
if keeping a string, convert it back to the raw 32-byte value (e.g., strip "0x"
and hex-decode/pad to 32 bytes) and return that byte slice instead of
[]byte(sid); update the return in the function that currently returns
addr.Bytes(), []byte(sid), aid, argBytes, nil to return the correctly decoded
[]byte streamID so canonicalization matches the on-chain bytes32.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
.golangci.yamlextensions/tn_utils/datapoints.goextensions/tn_utils/precompiles.goextensions/tn_utils/precompiles_decoding_test.gogo.modinternal/migrations/030-order-book-schema.sqlinternal/migrations/032-order-book-actions.sqlinternal/migrations/043-get-markets-by-stream.sqltests/streams/order_book/discovery_test.go
💤 Files with no reviewable changes (5)
- extensions/tn_utils/datapoints.go
- internal/migrations/030-order-book-schema.sql
- tests/streams/order_book/discovery_test.go
- extensions/tn_utils/precompiles_decoding_test.go
- internal/migrations/043-get-markets-by-stream.sql
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yaml (1)
29-29: Consider centralizing the Go version to avoid future drift.
1.25.3is duplicated in three jobs. Defining a workflow-level env var (for exampleGO_VERSION) would make upgrades safer and simpler.♻️ Suggested cleanup
name: CI +env: + GO_VERSION: '1.25.3' on: workflow_call: @@ - uses: actions/setup-go@v5 with: - go-version: '1.25.3' + go-version: ${{ env.GO_VERSION }} cache: false @@ - name: Setup Go uses: actions/setup-go@v5 with: - go-version: '1.25.3' + go-version: ${{ env.GO_VERSION }} @@ - name: Setup Go uses: actions/setup-go@v5 with: - go-version: '1.25.3' + go-version: ${{ env.GO_VERSION }}Also applies to: 63-63, 229-229
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml at line 29, Centralize the duplicated Go version by adding a workflow-level env var (e.g., env: GO_VERSION: '1.25.3') and update each job's go-version entries to reference that variable (replace occurrences of go-version: '1.25.3' with go-version: ${{ env.GO_VERSION }}); locate and change the go-version keys in all jobs that currently hardcode '1.25.3' so future upgrades only require changing the single GO_VERSION value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/ci.yaml:
- Line 29: Centralize the duplicated Go version by adding a workflow-level env
var (e.g., env: GO_VERSION: '1.25.3') and update each job's go-version entries
to reference that variable (replace occurrences of go-version: '1.25.3' with
go-version: ${{ env.GO_VERSION }}); locate and change the go-version keys in all
jobs that currently hardcode '1.25.3' so future upgrades only require changing
the single GO_VERSION value.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolves: https://github.com/truflation/website/issues/3382
Summary by CodeRabbit
Removals
Tests
Chores