chore: add precompile to unpack market stream and provider#1325
Merged
Conversation
Time Submission Status
You can submit time with the command. Example: See available commands to help comply with our Guidelines. |
📝 WalkthroughWalkthroughAdds a new precompile method 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://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/streams/order_book/fee_distribution_test.go (1)
146-150: Extract proposer key setup into a test helper.The same leader-key generation block is repeated in five places. A helper will reduce copy/paste and keep proposer context setup consistent.
Also applies to: 335-339, 480-484, 610-614, 741-745
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/streams/order_book/fee_distribution_test.go` around lines 146 - 150, Extract the repeated leader-key generation into a test helper (e.g., NewTestProposerPub or SetupProposerPubKey) that calls crypto.GenerateSecp256k1Key(nil), checks the error with require.NoError, and returns the typed *crypto.Secp256k1PublicKey (instead of repeating the pubGeneric, pub cast). Replace the five inlined blocks (the occurrences that create pubGeneric via crypto.GenerateSecp256k1Key and cast to *crypto.Secp256k1PublicKey) with a single call to this helper so tests use a consistent proposer key setup and avoid duplication.tests/streams/order_book/fee_distribution_audit_test.go (1)
533-534: Use the audited validator total instead of duplicating DP share.Line 533 reconstructs total fees with
infraSharetwice. SincetotalValFeesStris already parsed, use it directly so the integrity check follows audited values exactly.🔍 More robust total reconstruction
- totalFeesFromAudit := new(big.Int).Add(totalLPFeesFromAudit, new(big.Int).Add(infraShare, infraShare)) + valShare, _ := new(big.Int).SetString(totalValFeesStr, 10) + totalFeesFromAudit := new(big.Int).Add(totalLPFeesFromAudit, new(big.Int).Add(infraShare, valShare))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/streams/order_book/fee_distribution_audit_test.go` around lines 533 - 534, The test reconstructs totalFeesFromAudit by adding totalLPFeesFromAudit and infraShare twice; instead use the audited validator total parsed from totalValFeesStr so the assertion uses audited values exactly. Locate the variables totalFeesFromAudit, totalLPFeesFromAudit and infraShare and replace the duplicated infraShare addition with the parsed validator total (the big.Int value you derived from totalValFeesStr, e.g., totalValFees) so totalFeesFromAudit = totalLPFeesFromAudit + totalValFees before comparing to totalFees.tests/streams/order_book/matching_engine_test.go (1)
36-39: Extract zero-address fallback into one helper to avoid drift.The same fallback block is duplicated in two functions. A tiny helper will keep both paths in sync when this rule changes.
♻️ Suggested refactor
+func normalizedFromAddress(wallet string) string { + from := "0x0000000000000000000000000000000000000000" + if wallet == from { + return "0x0000000000000000000000000000000000000001" + } + return from +} ... - from := "0x0000000000000000000000000000000000000000" - if wallet == from { - from = "0x0000000000000000000000000000000000000001" - } + from := normalizedFromAddress(wallet) ... - from := "0x0000000000000000000000000000000000000000" - if wallet == from { - from = "0x0000000000000000000000000000000000000001" - } + from := normalizedFromAddress(wallet)Also applies to: 97-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/streams/order_book/matching_engine_test.go` around lines 36 - 39, Extract the duplicated zero-address fallback into a small helper (e.g., ensureNonZeroAddress(addr string) string or normalizeZeroAddress) that returns the provided address unless it equals "0x0000000000000000000000000000000000000000", in which case it returns "0x0000000000000000000000000000000000000001"; replace the inline checks that set from := "..."; if wallet == from { from = "0x...1" } with a call to this helper (e.g., from = ensureNonZeroAddress(wallet) or from = ensureNonZeroAddress(from)) in both places where the block currently appears so both paths use the same logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/streams/order_book/fee_distribution_test.go`:
- Around line 794-797: The test currently uses a permissive assertion comparing
'dist' and 'expectedDist' with require.True(dist.Cmp(expectedDist) >= 0),
allowing over-payments; change the assertion to require an exact match by
asserting the comparison equals zero (i.e., dist.Cmp(expectedDist) == 0) so the
resolved branch fails on any over- or under-distribution, and update the failure
message to reflect an exact-equality expectation; locate this check in
fee_distribution_test.go around the variables 'dist' and 'expectedDist' and
replace the require.True call accordingly.
---
Nitpick comments:
In `@tests/streams/order_book/fee_distribution_audit_test.go`:
- Around line 533-534: The test reconstructs totalFeesFromAudit by adding
totalLPFeesFromAudit and infraShare twice; instead use the audited validator
total parsed from totalValFeesStr so the assertion uses audited values exactly.
Locate the variables totalFeesFromAudit, totalLPFeesFromAudit and infraShare and
replace the duplicated infraShare addition with the parsed validator total (the
big.Int value you derived from totalValFeesStr, e.g., totalValFees) so
totalFeesFromAudit = totalLPFeesFromAudit + totalValFees before comparing to
totalFees.
In `@tests/streams/order_book/fee_distribution_test.go`:
- Around line 146-150: Extract the repeated leader-key generation into a test
helper (e.g., NewTestProposerPub or SetupProposerPubKey) that calls
crypto.GenerateSecp256k1Key(nil), checks the error with require.NoError, and
returns the typed *crypto.Secp256k1PublicKey (instead of repeating the
pubGeneric, pub cast). Replace the five inlined blocks (the occurrences that
create pubGeneric via crypto.GenerateSecp256k1Key and cast to
*crypto.Secp256k1PublicKey) with a single call to this helper so tests use a
consistent proposer key setup and avoid duplication.
In `@tests/streams/order_book/matching_engine_test.go`:
- Around line 36-39: Extract the duplicated zero-address fallback into a small
helper (e.g., ensureNonZeroAddress(addr string) string or normalizeZeroAddress)
that returns the provided address unless it equals
"0x0000000000000000000000000000000000000000", in which case it returns
"0x0000000000000000000000000000000000000001"; replace the inline checks that set
from := "..."; if wallet == from { from = "0x...1" } with a call to this helper
(e.g., from = ensureNonZeroAddress(wallet) or from = ensureNonZeroAddress(from))
in both places where the block currently appears so both paths use the same
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40daa66f-ddd3-4c13-8555-e11dd6f3e2bd
📒 Files selected for processing (5)
extensions/tn_utils/precompiles.gotests/streams/order_book/fee_distribution_audit_test.gotests/streams/order_book/fee_distribution_test.gotests/streams/order_book/lp_rewards_config_test.gotests/streams/order_book/matching_engine_test.go
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/streams/order_book/fee_distribution_audit_test.go`:
- Around line 169-173: The SetString calls currently ignore the bool result
which can leave nil/zero values and cause panics; update the parsing where
new(big.Int).SetString is used (e.g., when creating totalLPFeesFromAudit from
totalLPFeesStr and when parsing detail.rewardAmount into amt inside the loop
that accumulates totalDistributed) to check the returned ok boolean, handle
parse failures (return test error or fail/assert), and avoid using the parsed
*big.Int if ok is false; apply the same pattern to the other SetString sites
mentioned (around lines 511-517 and 528-534).
- Around line 514-522: The test currently mutates the expected value by
detecting leader status from the observed delta (variables increase1 and
expectedIncrease1 using totalValFeesStr), which makes the assertion
self-fulfilling; remove the branch that computes expectedIncrease1WithVal and
updates expectedIncrease1 based on increase1. Instead, use a fixed expected
value for this test (do not derive leader status from increase1) or move the
validator-leader case into a dedicated test that explicitly sets/knows the
leader and asserts against expectedIncrease1WithVal; update assertions to
compare increase1 only to the predetermined expected value(s) and not to a value
discovered from the result.
In `@tests/streams/order_book/fee_distribution_test.go`:
- Around line 208-216: The test currently infers the leader role by comparing
totalToUsers and totalFees and then mutates expectedDist1, which can mask
routing bugs; instead determine the expected leader and payout path before
running assertions (compute a boolean like isLeader by inspecting test fixture
inputs or the leader-selection variables, not the resulting balances), then
build expectedDist1, expectedTotalToUsers (using lpShareTotal and infraShare)
deterministically based on that precomputed role, and finally assert exact
equality of totalToUsers and expected totals with require.Equal; update the
branches around totalToUsers/totalFees, the mutation of expectedDist1, and the
corresponding asserts (seen near variables totalToUsers, totalFees,
expectedDist1, infraShare, lpShareTotal) to follow this pattern and apply the
same change to the other occurrences noted (lines ~398-404, ~513-516, ~778-781).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a6c4f8d1-98c5-4885-a352-515373f0ad98
📒 Files selected for processing (4)
tests/streams/order_book/fee_distribution_audit_test.gotests/streams/order_book/fee_distribution_test.gotests/streams/order_book/matching_engine_test.gotests/streams/order_book/test_helpers_orderbook.go
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/streams/order_book/matching_engine_test.go
This was referenced Mar 5, 2026
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/3419
Summary by CodeRabbit
New Features
Improvements
Tests