Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 18 minutes and 51 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIntegrated chain-event tracking into the transaction fetcher to pre-filter recently confirmed transactions, reducing redundant fetches. Added bounds validation to trie range proofs to reject invalid key batches. Modified tx-fetcher initialization to support optional blockchain integration while maintaining backward compatibility. Changes
Sequence Diagram(s)sequenceDiagram
participant ChainEvent as Chain Events
participant TxFetcher as Transaction Fetcher
participant Cache as txOnChainCache (LRU)
participant Announcements as Incoming Announcements
participant TxPool as Transaction Pool
ChainEvent->>TxFetcher: New block (ChainEvent)
alt Parent hash changed (reorg)
TxFetcher->>Cache: Clear cache
end
TxFetcher->>Cache: Add block transaction hashes
Announcements->>TxFetcher: Notify(hashes)
TxFetcher->>Cache: Check if hash is cached
alt Hash in cache (on-chain confirmed)
TxFetcher->>TxFetcher: Skip + record txAnnounceOnchainMeter
else Hash not cached
TxFetcher->>TxPool: Process for fetch
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
Updated the transaction fetcher to utilize a new on-chain cache for recently confirmed transactions, improving efficiency in filtering announcements. The `hasTx` function has been simplified to only check the local txpool, while the fetcher now tracks confirmed transactions from chain events. Additionally, new tests have been added to verify the behavior of the fetcher with respect to on-chain events and cache reorganization.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@trie/proof_test.go`:
- Around line 1026-1062: The test TestRangeProofPathPrefix incorrectly asserts
that a trie containing kShort and kLong (where kLong extends kShort) is invalid;
instead update the test to expect VerifyRangeProof to succeed for these keys.
Specifically, in the block that calls VerifyRangeProof(hash, kShort, kLong,
[][]byte{kShort,kLong}, [][]byte{[]byte("short"),[]byte("long")}, proof),
replace the current error-expecting assertions with a check that err == nil
(i.e., fail the test if err != nil), and remove or change the subsequent
exact-error-string comparison; keep references to Trie.Update, Trie.Prove and
VerifyRangeProof so the test still builds and verifies a valid range proof.
- Around line 941-968: The tests "first_before_start" and "last_after_end" rely
on wraparound-prone increseKey/decreseKey; replace those with deterministic
neighboring sorted keys from newKeysVals(): for the "first_before_start" case
set firstKey to common.CopyBytes(keys[1]) (and keep lastKey as keys[len-1]) so
the returned keys[0] is strictly before the requested start, and for
"last_after_end" set lastKey to common.CopyBytes(keys[len-2]) (and keep firstKey
as keys[0]) so the returned keys[len-1] is strictly after the requested end;
ensure you only use these when len(keys) > 1 and update the calls to
VerifyRangeProof accordingly (refer to functions newKeysVals, VerifyRangeProof
and the test names).
In `@trie/proof.go`:
- Around line 486-491: The current check in VerifyRangeProof (the
bytes.HasPrefix(keys[i+1], keys[i]) branch) incorrectly rejects valid MPT proofs
where a shorter key is represented via a branch value slot; remove the
unconditional prefix rejection and instead enforce a precondition for callers
that genuinely require fixed-width keys (e.g., check that keys[i] and keys[i+1]
have equal length before treating prefixes as an error). Update VerifyRangeProof
to either accept prefix relationships or only return the "range contains path
prefixes" error when a caller-specified fixed-length/same-length invariant is
violated (add an explicit length check or require the caller to assert
fixed-length keys prior to calling VerifyRangeProof). Ensure references:
VerifyRangeProof, keys slice, and the bytes.HasPrefix check are changed
accordingly.
🪄 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: ca83feba-0c8c-4eac-a39c-b3111189adc1
📒 Files selected for processing (5)
eth/fetcher/tx_fetcher.goeth/fetcher/tx_fetcher_test.goeth/handler.gotrie/proof.gotrie/proof_test.go
…y tests
- Narrow the bytes.HasPrefix guard to same-length key pairs only; shorter
keys that are byte-prefixes of longer keys are valid MPT entries (each
leaf nibble path carries a terminator) and must not be rejected.
- Replace wraparound-prone increseKey/decreseKey calls in the
first_before_start and last_after_end subtests with deterministic
neighboring sorted keys from newKeysVals().
- Update TestRangeProofPathPrefix to reflect the new rejection reason
("inconsistent edge keys") now that the false path-prefix error is gone.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary by CodeRabbit
New Features
Bug Fixes