Open
Conversation
Contributor
WalkthroughAdds a new detailed documentation file for the analyze-unverified-contract command and two new exported bash utilities for resolving selectors via 4byte.directory, including validation, batching, dependency checks, and request throttling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.cursor/commands/analyze-unverified-contract.md:
- Line 62: The comment incorrectly labels selector 0x4e487b71 as Error(string) —
update the text to correctly map selectors: change the exclusion list to
`0xffffffff` (sentinel), `0x4e487b71` (Panic(uint256)) and, if intending to
exclude Error(string), add `0x08c379a0` (Error(string)); edit the line
containing those selectors so it explicitly lists the correct
selector-to-signature mappings (`0x4e487b71` → Panic(uint256), `0x08c379a0` →
Error(string)) to avoid misleading users of the analyze-unverified-contract
command.
- Line 44: Update the install command string that currently reads "curl -L
http://get.heimdall.rs | bash" to use HTTPS ("curl -L https://get.heimdall.rs |
bash") in the analyze-unverified-contract instruction text so the installer is
fetched securely; keep the rest of the message unchanged and ensure any prompt
that offers to continue after install references the updated HTTPS URL.
In `@script/playgroundHelpers.sh`:
- Around line 2076-2084: The RESPONSE variable may contain non-JSON which makes
COUNT empty when parsing with jq, so update the selector-resolving logic (around
FOURBYTE_URL, RESPONSE, SELECTOR, COUNT) to detect jq parse failure and treat
that as "no match": after capturing RESPONSE, attempt to parse .count with jq
but check jq's exit status or whether COUNT is empty/null, and if jq fails or
COUNT is empty/null, print the "(no match)" line and return; this prevents
falling through into signature extraction on invalid API responses and keeps the
existing behavior for valid JSON.
🧹 Nitpick comments (2)
.cursor/commands/analyze-unverified-contract.md (1)
74-83: Section 7 sub-labels are non-sequential (D, B, C, A) — consider relabeling for clarity.The labels jump around (D → B → C → A) which is confusing even though the ordering by speed is intentional. Consider using numbered sub-steps (7.1, 7.2, 7.3, 7.4) or sequential letters (A, B, C, D) reordered to match the intended priority.
script/playgroundHelpers.sh (1)
2122-2128: Invalid selectors passed as arguments are silently discarded.When selectors are provided as positional arguments, invalid ones are skipped without any warning. A typo like
0x1234567gwould be silently ignored, which could confuse users who expect results for every argument.Proposed fix — emit a warning for skipped selectors
local ARG for ARG in "$@"; do if isValidSelector "$ARG"; then SELECTORS+=("$ARG") + else + warning "decodeSelectors4byte: skipping invalid selector '$ARG'" fi done
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
script/playgroundHelpers.sh (2)
2115-2159: Deduplicate selectors before querying to reduce redundant API hits.Selectors from
--stdinand positional args can contain duplicates, causing unnecessary requests and extra delay/rate-limit risk.♻️ Proposed fix
if [[ ${`#SELECTORS`[@]} -eq 0 ]]; then warning "decodeSelectors4byte: no selectors to decode" return 0 fi + + # Deduplicate while preserving first-seen order + mapfile -t SELECTORS < <(printf '%s\n' "${SELECTORS[@]}" | awk '!seen[$0]++') local SELECTOR for SELECTOR in "${SELECTORS[@]}"; do decodeSelector4byte "$SELECTOR" sleep "$DELAY_SEC"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/playgroundHelpers.sh` around lines 2115 - 2159, The SELECTORS array can contain duplicates from --stdin, positional args, or the default file, causing redundant decodeSelector4byte calls; after populating SELECTORS (before the check for empty and the for-loop invoking decodeSelector4byte), deduplicate its entries (e.g., use an associative array to track seen selectors or pipe through sort -u) and replace SELECTORS with the unique list so each selector is queried only once and the sleep/DELAY_SEC is not wasted on duplicates.
2076-2079: Add timeout safeguards for the 4byte API call for resilience.While
curl -sS -Lwithout timeout can hang on transient network issues, the.cursor/rules/300-bash.mdcguideline cited ("implement retries for RPC-sensitive steps") specifically applies to blockchain RPC calls, not generic HTTP APIs like 4byte.directory. The current code does include error handling with a fallback (|| echo '{"count":0}'), so it's not unprotected.That said, adding connection and max-time timeouts would still improve robustness and prevent indefinite hangs during batch operations:
🔧 Suggested enhancement
- RESPONSE=$(curl -sS -L "${FOURBYTE_URL}/?hex_signature=${SELECTOR}" 2>/dev/null || echo '{"count":0}') + RESPONSE=$(curl -sS -L \ + --connect-timeout 5 \ + --max-time 15 \ + "${FOURBYTE_URL}/?hex_signature=${SELECTOR}" 2>/dev/null || echo '{"count":0}')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/playgroundHelpers.sh` around lines 2076 - 2079, The 4byte API curl call that sets RESPONSE can hang without timeouts; update the curl invocation in the RESPONSE assignment (the call using FOURBYTE_URL and SELECTOR) to include connection and overall timeouts (e.g., --connect-timeout and --max-time) and optionally lightweight retry flags (e.g., --retry, --retry-delay) so the call fails fast and falls back to the existing || echo '{"count":0}'; ensure you only change the curl options in that RESPONSE line and preserve the existing fallback behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.cursor/commands/analyze-unverified-contract.md:
- Around line 68-83: The documentation lacks an explicit statement that running
decodeSelectors4byte with no arguments will read and update opcodes-selectors.md
by default; update the text around the decodeSelectors4byte call to explicitly
state that the tool uses opcodes-selectors.md as the default input/output file
(and that you can supply a custom filename to override), and mention the
expected error (“file not found”) if that default file is missing; reference the
decodeSelectors4byte helper and the opcodes-selectors.md filename so readers
know the exact default behavior and how to pass a custom selector file.
- Around line 65-67: The selector extraction regex 'PUSH4 [0-9a-f]{8}' only
matches lowercase hex and drops uppercase A–F; update the pattern in the command
(the string "PUSH4 [0-9a-f]{8}") to accept both cases (e.g. 'PUSH4
[0-9a-fA-F]{8}' or use a POSIX class like 'PUSH4 [[:xdigit:]]{8}') so uppercase
hex selectors are included, leaving the rest of the pipeline (awk, sort, grep -v
filters) unchanged.
In `@script/playgroundHelpers.sh`:
- Around line 2098-2153: Update the leading comment for the decodeSelectors4byte
function to match its actual return behavior: it returns 1 when required
dependencies or the default file are missing (error paths where error
"decodeSelectors4byte: ..." is called), and returns 0 on success or when there
are no selectors to decode (the warning path that currently returns 0). Edit the
comment line that currently reads "Returns: 0 on success; 1 if no selectors or
dependency missing" to reflect "Returns: 0 on success or when no selectors; 1 on
missing dependency or default file error" so it aligns with the implementation
(referencing decodeSelectors4byte, SELECTORS, and the early dependency checks).
---
Nitpick comments:
In `@script/playgroundHelpers.sh`:
- Around line 2115-2159: The SELECTORS array can contain duplicates from
--stdin, positional args, or the default file, causing redundant
decodeSelector4byte calls; after populating SELECTORS (before the check for
empty and the for-loop invoking decodeSelector4byte), deduplicate its entries
(e.g., use an associative array to track seen selectors or pipe through sort -u)
and replace SELECTORS with the unique list so each selector is queried only once
and the sleep/DELAY_SEC is not wasted on duplicates.
- Around line 2076-2079: The 4byte API curl call that sets RESPONSE can hang
without timeouts; update the curl invocation in the RESPONSE assignment (the
call using FOURBYTE_URL and SELECTOR) to include connection and overall timeouts
(e.g., --connect-timeout and --max-time) and optionally lightweight retry flags
(e.g., --retry, --retry-delay) so the call fails fast and falls back to the
existing || echo '{"count":0}'; ensure you only change the curl options in that
RESPONSE line and preserve the existing fallback behavior.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 66d9dadb-c3f7-42d6-83aa-32cb665c6c93
📒 Files selected for processing (2)
.cursor/commands/analyze-unverified-contract.mdscript/playgroundHelpers.sh
Comment on lines
+65
to
+67
| ```bash | ||
| grep -oE 'PUSH4 [0-9a-f]{8}' opcodes.txt | awk '{print "0x"$2}' | sort -u | grep -v '0xffffffff' | grep -v '0x4e487b71' | grep -v '0x08c379a0' | ||
| ``` |
Contributor
There was a problem hiding this comment.
Selector extraction example can miss uppercase PUSH4 values.
The regex only matches lowercase hex, so valid selectors with A-F get dropped from the report.
🔧 Proposed fix
- grep -oE 'PUSH4 [0-9a-f]{8}' opcodes.txt | awk '{print "0x"$2}' | sort -u | grep -v '0xffffffff' | grep -v '0x4e487b71' | grep -v '0x08c379a0'
+ grep -oEi 'PUSH4 [0-9a-f]{8}' opcodes.txt | awk '{print "0x"tolower($2)}' | sort -u | grep -v '0xffffffff' | grep -v '0x4e487b71' | grep -v '0x08c379a0'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.cursor/commands/analyze-unverified-contract.md around lines 65 - 67, The
selector extraction regex 'PUSH4 [0-9a-f]{8}' only matches lowercase hex and
drops uppercase A–F; update the pattern in the command (the string "PUSH4
[0-9a-f]{8}") to accept both cases (e.g. 'PUSH4 [0-9a-fA-F]{8}' or use a POSIX
class like 'PUSH4 [[:xdigit:]]{8}') so uppercase hex selectors are included,
leaving the rest of the pipeline (awk, sort, grep -v filters) unchanged.
Comment on lines
+68
to
+83
| - **Output**: Write to e.g. `opcodes-selectors.md` or `<contract>-selectors.md` (list or table Selector | Signature). | ||
|
|
||
| --- | ||
|
|
||
| ## 7. Enrich selectors with signatures | ||
|
|
||
| **Use this order for speed**: resolve via known + local + cast first (no network delay), then **always** run the 4byte.directory step for every selector still unresolved after cast. | ||
|
|
||
| **D. Known selectors** (instant) — 0x01ffc9a7→supportsInterface(bytes4), 0x1626ba7e→isValidSignature(bytes32,bytes), 0x150b7a02→onERC721Received(…), 0xf23a6e61→onERC1155Received(…), 0xbc197c81→onERC1155BatchReceived(…), 0x52d1902d→proxiableUUID(), 0x4f1ef286→upgradeToAndCall(address,bytes), 0x3f707e6b→execute((address,uint256,bytes)[]), 0xb61d27f6→execute(address,uint256,bytes). Add common ERC20/ERC721 (e.g. balanceOf, transfer, approve) if useful. | ||
|
|
||
| **B. Local repo** (instant) — `out/<ContractName>.sol/<ContractName>.json` → key `methodIdentifiers`. `grep -r "<selector>" out/` or jq over methodIdentifiers. | ||
|
|
||
| **C. cast** (fast, no rate limit) — `cast 4byte <selector>` returns signature; use for each unresolved selector before hitting the API. | ||
|
|
||
| **A. 4byte.directory (mandatory for unresolved)** — For every selector still showing no signature after D/B/C, query 4byte.directory and merge any results into the selectors file. **Preferred**: source `script/helperFunctions.sh` and `script/playgroundHelpers.sh`, then run `decodeSelectors4byte` with no args (reads from the selectors file) or pass only the unresolved selectors; use `FOURBYTE_DELAY=0.3`–0.5. **Fallback**: if sourcing the repo scripts fails (e.g. in a minimal env), call the API directly: `GET https://www.4byte.directory/api/v1/signatures/?hex_signature=0x<selector>`, parse `results[].text_signature`, use ~0.3s delay between requests, and update the selectors file with any new signatures. Selectors that remain unknown after 4byte stay as "—" in the table. | ||
|
|
Contributor
There was a problem hiding this comment.
Clarify default-file behavior when using decodeSelectors4byte with no args.
Line 68 allows custom selector filenames, but no-arg decodeSelectors4byte reads opcodes-selectors.md by default. Call this out explicitly to avoid a confusing “file not found” path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.cursor/commands/analyze-unverified-contract.md around lines 68 - 83, The
documentation lacks an explicit statement that running decodeSelectors4byte with
no arguments will read and update opcodes-selectors.md by default; update the
text around the decodeSelectors4byte call to explicitly state that the tool uses
opcodes-selectors.md as the default input/output file (and that you can supply a
custom filename to override), and mention the expected error (“file not found”)
if that default file is missing; reference the decodeSelectors4byte helper and
the opcodes-selectors.md filename so readers know the exact default behavior and
how to pass a custom selector file.
Comment on lines
+2098
to
+2153
| # Returns: 0 on success; 1 if no selectors or dependency missing | ||
| # Example: decodeSelectors4byte 0x8388464e 0x12345678 | ||
| # Example: decodeSelectors4byte | ||
| # Example: grep -oE '0x[0-9a-f]{8}' opcodes-selectors.md | decodeSelectors4byte --stdin | ||
| function decodeSelectors4byte() { | ||
| local DELAY_SEC="${FOURBYTE_DELAY:-0.3}" | ||
| local SELECTORS=() | ||
|
|
||
| command -v curl >/dev/null 2>&1 || { | ||
| error "decodeSelectors4byte: curl is required" | ||
| return 1 | ||
| } | ||
| command -v jq >/dev/null 2>&1 || { | ||
| error "decodeSelectors4byte: jq is required" | ||
| return 1 | ||
| } | ||
|
|
||
| if [[ "${1:-}" == "--stdin" ]]; then | ||
| local LINE | ||
| while read -r LINE; do | ||
| if isValidSelector "$LINE"; then | ||
| SELECTORS+=("$LINE") | ||
| fi | ||
| done | ||
| elif [[ $# -gt 0 ]]; then | ||
| local ARG | ||
| for ARG in "$@"; do | ||
| if isValidSelector "$ARG"; then | ||
| SELECTORS+=("$ARG") | ||
| fi | ||
| done | ||
| else | ||
| local REPO_ROOT | ||
| REPO_ROOT=$(getContractsDirectory 2>/dev/null) || true | ||
| if [[ -z "$REPO_ROOT" ]]; then | ||
| local SCRIPT_DIR | ||
| SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]:-.}")" && pwd) | ||
| REPO_ROOT=$(cd "$SCRIPT_DIR/.." && pwd) | ||
| fi | ||
| local MD="${REPO_ROOT}/opcodes-selectors.md" | ||
| if [[ ! -f "$MD" ]]; then | ||
| error "decodeSelectors4byte: no selectors provided and default file not found: $MD" | ||
| error "Usage: decodeSelectors4byte [--stdin] [SELECTOR ...]" | ||
| return 1 | ||
| fi | ||
| while read -r LINE; do | ||
| if [[ "$LINE" =~ (0x[0-9a-fA-F]{8}) ]]; then | ||
| SELECTORS+=("${BASH_REMATCH[1]}") | ||
| fi | ||
| done < <(grep -oE '0x[0-9a-fA-F]{8}' "$MD" | sort -u) | ||
| fi | ||
|
|
||
| if [[ ${#SELECTORS[@]} -eq 0 ]]; then | ||
| warning "decodeSelectors4byte: no selectors to decode" | ||
| return 0 | ||
| fi |
Contributor
There was a problem hiding this comment.
Fix return-code docs to match implementation.
The comment says “Returns: 0 on success; 1 if no selectors…”, but no selectors currently returns 0 (Line 2152).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@script/playgroundHelpers.sh` around lines 2098 - 2153, Update the leading
comment for the decodeSelectors4byte function to match its actual return
behavior: it returns 1 when required dependencies or the default file are
missing (error paths where error "decodeSelectors4byte: ..." is called), and
returns 0 on success or when there are no selectors to decode (the warning path
that currently returns 0). Edit the comment line that currently reads "Returns:
0 on success; 1 if no selectors or dependency missing" to reflect "Returns: 0 on
success or when no selectors; 1 on missing dependency or default file error" so
it aligns with the implementation (referencing decodeSelectors4byte, SELECTORS,
and the early dependency checks).
mirooon
reviewed
Mar 16, 2026
| @@ -0,0 +1,134 @@ | |||
| --- | |||
Contributor
There was a problem hiding this comment.
nice! Imo its worth to add as a fallback for unverified contracts in analyze-tx.md command. wdyt?
| - Run `command -v heimdall` (or `which heimdall`). | ||
| - If **not found**: Tell the user: "Heimdall is not installed. Install with: `curl -L https://get.heimdall.rs | bash`, then in a new terminal run `bifrost`." Offer to continue after they install (reference the same HTTPS install URL if they ask how to install). Do **not** proceed with disassemble/decompile until heimdall is available. | ||
|
|
||
| --- |
Contributor
There was a problem hiding this comment.
I would add here proxy check to make sure we are not analyzing proxy contract but the implementation contracts. Something like this:
- Before decompiling, check if the contract is an EIP-1967 proxy by reading its implementation slot:
`cast storage <ADDRESS> 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc --rpc-url <RPC_URL>`
- If the result is a non-zero address, **this is a proxy**. Use this new implementation address for the Heimdall target in the steps below to extract the actual logic.
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
3 participants
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.
Why did I implement it this way?
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)