add isStartBlock and findDeployBlock to locate contract deploy blocks#5
add isStartBlock and findDeployBlock to locate contract deploy blocks#5thedavidmeister merged 2 commits intomainfrom
Conversation
Binary searches fork history via vm.rollFork to find the first block where a contract with the expected code hash appears. isStartBlock validates that a block is the deployment boundary by checking the code hash exists at the block but not at block - 1. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdded two new errors and two internal utility functions to LibRainDeploy for deploy-trace capabilities. The Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Suite
participant FindDeploy as findDeployBlock()
participant IsStart as isStartBlock()
participant Vm as Forge VM
participant History as Block History
Test->>FindDeploy: findDeployBlock(target, codeHash, startBlock)
FindDeploy->>Vm: Check code exists at current block
Vm-->>FindDeploy: Code present/absent
FindDeploy->>Vm: Check code hash matches at current block
Vm-->>FindDeploy: Hash valid/invalid
alt Binary Search Loop
FindDeploy->>IsStart: isStartBlock(mid block)
IsStart->>Vm: Fork to mid - 1 block
IsStart->>Vm: Get code hash at mid - 1
IsStart->>Vm: Fork to mid block
IsStart->>Vm: Get code hash at mid
IsStart->>Vm: Restore original fork
Vm-->>IsStart: Comparison result
IsStart-->>FindDeploy: Is start block?
end
FindDeploy-->>Test: Deploy block number or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 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)
📝 Coding Plan for PR comments
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 |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/LibRainDeploy.sol`:
- Around line 91-96: The docstring for the deployment search claims the result
"is validated via `isStartBlock`" but the function `findDeployBlock` doesn't
call `isStartBlock`; update `findDeployBlock` to invoke
`isStartBlock(foundBlock, target)` (or the appropriate signature) after locating
the candidate block and before returning, and revert or return an error if
`isStartBlock` returns false; ensure the fork is still restored to its original
block number before reverting/returning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f49d362-2622-4b2a-9194-7d843fb11a57
📒 Files selected for processing (2)
src/lib/LibRainDeploy.soltest/src/lib/LibRainDeploy.t.sol
| /// Finds the block number at which a contract was first deployed by binary | ||
| /// searching the fork history. Requires an active fork with archive access | ||
| /// back to `startBlock`. The fork is restored to its original block | ||
| /// number before returning. The target's code hash is verified against the | ||
| /// expected value before searching. The result is validated via | ||
| /// `isStartBlock`. |
There was a problem hiding this comment.
Documentation states validation that isn't performed.
The docstring says "The result is validated via isStartBlock" but the implementation doesn't call isStartBlock. The tests call it separately for verification (see testFindDeployBlockZoltuFactory). Either add the validation call or update the docstring to clarify that callers should validate the result.
📝 Suggested docstring fix
/// Finds the block number at which a contract was first deployed by binary
/// searching the fork history. Requires an active fork with archive access
/// back to `startBlock`. The fork is restored to its original block
/// number before returning. The target's code hash is verified against the
-/// expected value before searching. The result is validated via
-/// `isStartBlock`.
+/// expected value before searching. Callers can validate the result via
+/// `isStartBlock` if needed.📝 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.
| /// Finds the block number at which a contract was first deployed by binary | |
| /// searching the fork history. Requires an active fork with archive access | |
| /// back to `startBlock`. The fork is restored to its original block | |
| /// number before returning. The target's code hash is verified against the | |
| /// expected value before searching. The result is validated via | |
| /// `isStartBlock`. | |
| /// Finds the block number at which a contract was first deployed by binary | |
| /// searching the fork history. Requires an active fork with archive access | |
| /// back to `startBlock`. The fork is restored to its original block | |
| /// number before returning. The target's code hash is verified against the | |
| /// expected value before searching. Callers can validate the result via | |
| /// `isStartBlock` if needed. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/LibRainDeploy.sol` around lines 91 - 96, The docstring for the
deployment search claims the result "is validated via `isStartBlock`" but the
function `findDeployBlock` doesn't call `isStartBlock`; update `findDeployBlock`
to invoke `isStartBlock(foundBlock, target)` (or the appropriate signature)
after locating the candidate block and before returning, and revert or return an
error if `isStartBlock` returns false; ensure the fork is still restored to its
original block number before reverting/returning.
1 participant
Binary searches fork history via vm.rollFork to find the first block where a contract with the expected code hash appears. isStartBlock validates that a block is the deployment boundary by checking the code hash exists at the block but not at block - 1.
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Tests