Conversation
WalkthroughThis PR adds documentation for the analyze-unverified-contract command workflow and introduces two new bash utility functions for 4byte selector decoding. The documentation outlines contract analysis steps including RPC resolution, disassembly, and report generation. The utilities validate selectors and batch-query the 4byte.directory API with configurable delays. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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: 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
| ## 4. Heimdall installation check | ||
|
|
||
| - Run `command -v heimdall` (or `which heimdall`). | ||
| - If **not found**: Tell the user: "Heimdall is not installed. Install with: `curl -L http://get.heimdall.rs | bash`, then in a new terminal run `bifrost`." Offer to continue after they install. Do **not** proceed with disassemble/decompile until heimdall is available. |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
heimdall-rs install https get.heimdall.rs
💡 Result:
To install heimdall-rs via the get.heimdall.rs installer (installs bifrost, then you run bifrost to build/install Heimdall):
curl -L https://get.heimdall.rs | bashThen open a new terminal and run:
bifrost[1] [2]
Change HTTP to HTTPS in the Heimdall install URL.
The install command should use https://get.heimdall.rs instead of http://get.heimdall.rs to avoid MITM attacks when piping the script into bash.
🤖 Prompt for AI Agents
In @.cursor/commands/analyze-unverified-contract.md at 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.
| ## 6. Extract function selectors from opcodes | ||
|
|
||
| - **Pattern**: `PUSH4 <8 hex digits>` (e.g. `PUSH4 8388464e`). | ||
| - **Exclude**: `0xffffffff` (sentinel), `0x4e487b71` (Error(string)). |
There was a problem hiding this comment.
Incorrect selector comment: 0x4e487b71 is Panic(uint256), not Error(string).
Error(string) is 0x08c379a0. The selector 0x4e487b71 corresponds to Panic(uint256). This would mislead anyone using this command to analyze contracts.
Proposed fix
-- **Exclude**: `0xffffffff` (sentinel), `0x4e487b71` (Error(string)).
+- **Exclude**: `0xffffffff` (sentinel), `0x4e487b71` (Panic(uint256)).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Exclude**: `0xffffffff` (sentinel), `0x4e487b71` (Error(string)). | |
| - **Exclude**: `0xffffffff` (sentinel), `0x4e487b71` (Panic(uint256)). |
🤖 Prompt for AI Agents
In @.cursor/commands/analyze-unverified-contract.md at 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.
| local FOURBYTE_URL="${FOURBYTE_URL:-https://www.4byte.directory/api/v1/signatures}" | ||
| local RESPONSE | ||
| RESPONSE=$(curl -sS -L "${FOURBYTE_URL}/?hex_signature=${SELECTOR}" 2>/dev/null || echo '{"count":0}') | ||
| local COUNT | ||
| COUNT=$(echo "$RESPONSE" | jq -r '.count // 0') | ||
| if [[ "$COUNT" == "0" || "$COUNT" == "null" ]]; then | ||
| printf "%s\t%s\n" "$SELECTOR" "(no match)" | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
Empty COUNT from jq parse failure falls through incorrectly.
If the API returns non-JSON (e.g., rate-limit HTML or server error), jq -r '.count // 0' will fail and COUNT will be empty. The check on line 2081 doesn't cover that case, causing the function to fall through to the signatures extraction which will also produce garbage output.
Proposed fix
local RESPONSE
RESPONSE=$(curl -sS -L "${FOURBYTE_URL}/?hex_signature=${SELECTOR}" 2>/dev/null || echo '{"count":0}')
local COUNT
COUNT=$(echo "$RESPONSE" | jq -r '.count // 0')
- if [[ "$COUNT" == "0" || "$COUNT" == "null" ]]; then
+ if [[ -z "$COUNT" || "$COUNT" == "0" || "$COUNT" == "null" ]]; then
printf "%s\t%s\n" "$SELECTOR" "(no match)"
return 0
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local FOURBYTE_URL="${FOURBYTE_URL:-https://www.4byte.directory/api/v1/signatures}" | |
| local RESPONSE | |
| RESPONSE=$(curl -sS -L "${FOURBYTE_URL}/?hex_signature=${SELECTOR}" 2>/dev/null || echo '{"count":0}') | |
| local COUNT | |
| COUNT=$(echo "$RESPONSE" | jq -r '.count // 0') | |
| if [[ "$COUNT" == "0" || "$COUNT" == "null" ]]; then | |
| printf "%s\t%s\n" "$SELECTOR" "(no match)" | |
| return 0 | |
| fi | |
| local FOURBYTE_URL="${FOURBYTE_URL:-https://www.4byte.directory/api/v1/signatures}" | |
| local RESPONSE | |
| RESPONSE=$(curl -sS -L "${FOURBYTE_URL}/?hex_signature=${SELECTOR}" 2>/dev/null || echo '{"count":0}') | |
| local COUNT | |
| COUNT=$(echo "$RESPONSE" | jq -r '.count // 0') | |
| if [[ -z "$COUNT" || "$COUNT" == "0" || "$COUNT" == "null" ]]; then | |
| printf "%s\t%s\n" "$SELECTOR" "(no match)" | |
| return 0 | |
| fi |
🤖 Prompt for AI Agents
In `@script/playgroundHelpers.sh` around lines 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.
2 participants
Why did I implement it this way?
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)