Conversation
|
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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughReplaces local RSA/crypto signing and related deps with a git-based Changes
Sequence DiagramsequenceDiagram
participant Caller as Oracle Update Method
participant Signer as send_transaction(call)
participant LocalSigner as LocalWallet Signer
participant RemoteSigner as SignerClient
participant L2Provider as L2 Provider
Caller->>Signer: call: ContractCall<M,D>
activate Signer
Signer->>Signer: extract to, method_sig, calldata
alt ext_signer present
Signer->>RemoteSigner: sign(tx, method_sig)
activate RemoteSigner
RemoteSigner-->>Signer: signed_tx_bytes
deactivate RemoteSigner
else local signer
Signer->>LocalSigner: sign_transaction(tx)
activate LocalSigner
LocalSigner-->>Signer: signed_tx_bytes
deactivate LocalSigner
end
Signer->>L2Provider: send_raw_transaction(signed_tx)
activate L2Provider
L2Provider-->>Signer: tx_hash
deactivate L2Provider
Signer-->>Caller: Result<H256>
deactivate Signer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gas-oracle/app/src/da_scalar/l1_scalar.rs (1)
46-50:⚠️ Potential issue | 🟠 MajorFix the test to use
SignerClientinstead ofExternalSign.Line 50 changes the constructor parameter to
ext_signer: Option<SignerClient>, but the ignored test at lines 467-468 still instantiatesExternalSignand passes it toScalarUpdater::new(). SinceExternalSignis undefined in this file andcargo testcompiles ignored tests, this will cause a compilation failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gas-oracle/app/src/da_scalar/l1_scalar.rs` around lines 46 - 50, The ignored test still constructs an ExternalSign and passes it to ScalarUpdater::new, but the constructor now expects ext_signer: Option<SignerClient>; update the test to construct (or mock) a SignerClient and pass Some(signer_client) (or None if you want no external signer) to ScalarUpdater::new, replace all uses/imports of ExternalSign with SignerClient, and add any needed imports or helper creation code so the test compiles.
🧹 Nitpick comments (1)
gas-oracle/app/Cargo.toml (1)
26-26: Pinremote-signer-clientto an immutable revision.Leaving the signer client on a floating git HEAD means a lockfile refresh can silently pull different signing code. For a signing dependency, pinning
revortagkeeps builds reproducible.Suggested manifest change
-remote-signer-client = { git = "https://github.com/morph-l2/remote-signer-client", package = "remote-signer-client" } +remote-signer-client = { git = "https://github.com/morph-l2/remote-signer-client", rev = "<commit-sha>", package = "remote-signer-client" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gas-oracle/app/Cargo.toml` at line 26, The dependency declaration for remote-signer-client should be pinned to a specific git revision or tag instead of floating to HEAD; update the Cargo.toml entry for remote-signer-client (the dependency line referencing git = "https://github.com/morph-l2/remote-signer-client", package = "remote-signer-client") to include either rev = "<commit-sha>" or tag = "<tag-name>" so the crate resolves reproducibly and repeatably during builds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gas-oracle/app/src/gas_price_oracle.rs`:
- Around line 191-199: The code currently calls
SignerClient::new(...).map_err(...).unwrap(), causing a panic on failure; change
this to propagate the error from prepare_updater() by replacing unwrap() with
the ? operator (or returning the mapped anyhow! error via ?). Specifically,
update the call to SignerClient::new in prepare_updater to return its Result
(use ? after map_err or drop map_err and use .context(...) then ?), so failures
from SignerClient::new propagate instead of crashing.
In `@gas-oracle/app/src/signer.rs`:
- Around line 31-34: The current match on call.tx.to() falls back to
Address::zero(), which silently sends to the zero address; instead, detect
non-address targets and return an explicit error (or propagate one) when
call.tx.to() is None or NameOrAddress::Name. Replace the fallback in the code
that sets contract (the match on call.tx.to() producing Address::zero()) with
logic that returns a descriptive error (e.g., MissingCallTarget or
InvalidCallTarget) so the remote-sign path fails fast and lets callers/policy
checks handle missing/Name targets.
---
Outside diff comments:
In `@gas-oracle/app/src/da_scalar/l1_scalar.rs`:
- Around line 46-50: The ignored test still constructs an ExternalSign and
passes it to ScalarUpdater::new, but the constructor now expects ext_signer:
Option<SignerClient>; update the test to construct (or mock) a SignerClient and
pass Some(signer_client) (or None if you want no external signer) to
ScalarUpdater::new, replace all uses/imports of ExternalSign with SignerClient,
and add any needed imports or helper creation code so the test compiles.
---
Nitpick comments:
In `@gas-oracle/app/Cargo.toml`:
- Line 26: The dependency declaration for remote-signer-client should be pinned
to a specific git revision or tag instead of floating to HEAD; update the
Cargo.toml entry for remote-signer-client (the dependency line referencing git =
"https://github.com/morph-l2/remote-signer-client", package =
"remote-signer-client") to include either rev = "<commit-sha>" or tag =
"<tag-name>" so the crate resolves reproducibly and repeatably during builds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7e5e2a2-ee1b-4975-a9fa-9c269ba74b1d
⛔ Files ignored due to path filters (1)
gas-oracle/app/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
gas-oracle/app/Cargo.tomlgas-oracle/app/src/da_scalar/l1_scalar.rsgas-oracle/app/src/gas_price_oracle.rsgas-oracle/app/src/l1_base_fee.rsgas-oracle/app/src/lib.rsgas-oracle/app/src/signer.rs
💤 Files with no reviewable changes (1)
- gas-oracle/app/src/lib.rs
1 participant
update gas price oracle sign method
Summary by CodeRabbit
New Features
Chores