chore: restrict attestation to single-row actions only#1349
Merged
Conversation
Time Submission Status
You can submit time with the command. Example: See available commands to help comply with our Guidelines. |
📝 WalkthroughWalkthroughThis PR adds public attestation actions Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Facade as Public Facade
participant Check as Stream Checker
participant Prim as Primitive Action
participant Comp as Composed Aggregator
participant DB as Database
Client->>Facade: get_high_value($provider,$stream_id,$from,$to,$frozen_at)
Facade->>Facade: normalize $data_provider
Facade->>Check: is_primitive_stream($stream_id)?
alt primitive
Check-->>Facade: true
Facade->>Prim: call primitive get_high_value
Prim->>DB: authorize (is_allowed_to_read_core)
DB-->>Prim: auth result
Prim->>DB: query primitive_events within bounds
DB-->>Prim: deduplicated rows (ROW_NUMBER)
Prim-->>Facade: single (event_time,value)
else composed
Check-->>Facade: false
Facade->>Comp: iterate get_record(..., FALSE)
Comp->>DB: fetch composed records
DB-->>Comp: record rows
Comp->>Comp: compute max/min value & event_time
Comp-->>Facade: single (event_time,value) or NULL
end
Facade-->>Client: return result or NULL
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📝 Generate docstrings
🧪 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
🤖 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`:
- Around line 767-770: validate_attestation_date_range was updated to accept
actionIDs 10 and 11 (get_high_value/get_low_value) but getActionIDNumber still
maps only 1–9 causing compute_attestation_hash to return "unknown action";
update getActionIDNumber to include mappings for 10 -> get_high_value and 11 ->
get_low_value (or otherwise ensure compute_attestation_hash recognizes IDs 10
and 11) so the hash path includes these actions consistently with
validate_attestation_date_range and any callers of compute_attestation_hash.
In `@tests/streams/attestation/request_attestation_fee_test.go`:
- Around line 368-385: callRequestAttestationActionWithTimeRange currently
always builds args for get_high_value/get_low_value, which mismatches callers in
attestation_date_range_test.go that call get_record, get_index, and
get_change_over_time; update callRequestAttestationActionWithTimeRange to branch
on actionName and construct the correct ABI-shaped actionArgs per action (e.g.,
for "get_high_value"/"get_low_value" use [dataProvider, streamID, from, to,
frozen_at], for "get_record"/"get_index"/"get_change_over_time" build the
specific argument list those ABIs expect), or alternatively restrict this helper
to only high/low actions and update callers to use a different helper; implement
the branch by switching on actionName in
callRequestAttestationActionWithTimeRange and producing the appropriate
actionArgs before calling tn_utils.EncodeActionArgs and
callRequestAttestationWithBytes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b4c22208-dafd-4e6e-8bcc-d60a1b449ab1
📒 Files selected for processing (5)
extensions/tn_utils/precompiles.gointernal/migrations/045-high-low-attestation-actions.sqltests/streams/attestation/attestation_date_range_test.gotests/streams/attestation/request_attestation_fee_test.gotests/streams/attestation/test_helpers.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/streams/order_book/settlement_test.go (1)
706-709: Potential argument mismatch whenargsBytesisnil.The
encodeQueryComponentsForTestshelper (per relevant snippet attest_helpers_orderbook.go:143-200) uses default args withget_recordsemantics (6 args:data_provider, stream_id, from, to, frozen_at, use_cache) whenargsBytesisnil. However,get_last_recordexpects 5 args:(data_provider, stream_id, before, frozen_at, use_cache).While this test intentionally creates a market without a matching attestation (to verify the "no attestation" error path), the argument mismatch could cause issues if the validation logic ever inspects the args structure. Consider explicitly encoding the correct args for consistency:
🔧 Suggested fix
- fakeQueryComponents, err := encodeQueryComponentsForTests(dataProvider, streamID, "get_last_record", nil) + fakeArgsBytes, err := tn_utils.EncodeActionArgs([]any{ + dataProvider, streamID, int64(1500), nil, false, + }) + require.NoError(t, err) + fakeQueryComponents, err := encodeQueryComponentsForTests(dataProvider, streamID, "get_last_record", fakeArgsBytes)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/streams/order_book/settlement_test.go` around lines 706 - 709, The test calls encodeQueryComponentsForTests with argsBytes == nil which makes the helper default to the 6-arg get_record signature, but this test uses action "get_last_record" which expects 5 args; fix by constructing and passing explicit argsBytes for get_last_record (populate data_provider, stream_id, before, frozen_at, use_cache) and pass that into encodeQueryComponentsForTests instead of nil so the encoded query matches get_last_record semantics; locate the call site in settlement_test.go and update the argsBytes creation near where encodeQueryComponentsForTests is invoked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/streams/order_book/settlement_test.go`:
- Around line 706-709: The test calls encodeQueryComponentsForTests with
argsBytes == nil which makes the helper default to the 6-arg get_record
signature, but this test uses action "get_last_record" which expects 5 args; fix
by constructing and passing explicit argsBytes for get_last_record (populate
data_provider, stream_id, before, frozen_at, use_cache) and pass that into
encodeQueryComponentsForTests instead of nil so the encoded query matches
get_last_record semantics; locate the call site in settlement_test.go and update
the argsBytes creation near where encodeQueryComponentsForTests is invoked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0b3ae8cc-969c-4aff-8e21-6cb13224e259
📒 Files selected for processing (6)
extensions/tn_settlement/settlement_integration_test.goextensions/tn_utils/precompiles.gotests/streams/order_book/hash_compatibility_test.gotests/streams/order_book/order_events_test.gotests/streams/order_book/settlement_test.gotests/streams/transaction_events_ledger_test.go
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/3565
Summary by CodeRabbit
New Features
Bug Fixes
Tests