chore: Change optimism tag#426
Conversation
WalkthroughUpdated Optimism-related tagging from "optimism" to "optimism-based" across config, docs, and code. Config JSON tags changed for two networks; documentation updated to reference the new tag; EvmNetwork.is_optimism() now checks for "optimism-based". No API/signature changes. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App
participant Config as Network Config
participant EVM as EvmNetwork
participant Logic as Optimism-specific Logic
App->>Config: Load network definition
Config-->>EVM: tags = ["rollup","optimism-based"]
EVM->>EVM: is_optimism() checks for "optimism-based"
alt Tag present
EVM-->>Logic: Enable Optimism-path
Logic-->>App: Proceed with OP-specific behavior
else Tag absent
EVM-->>App: Use default EVM path
end
note over EVM,Logic: Tag key changed from "optimism" to "optimism-based"
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
config/networks/optimism.json (1)
26-27: Ensure OP Stack networks in this file carry the new tag where intended.Base and World Chain entries currently have only ["rollup"] (and ["deprecated","rollup"] in testnets). If they are meant to be treated as OP Stack networks by code paths keyed off is_optimism(), consider adding "optimism-based" to their tags for consistency with docs.
Example adjustments (illustrative snippets):
// base "tags": ["rollup", "optimism-based"] // base-sepolia "tags": ["deprecated", "rollup", "optimism-based"] // world-chain "tags": ["rollup", "optimism-based"] // world-chain-sepolia "tags": ["deprecated", "rollup", "optimism-based"]Note: If Unichain is also OP Stack in your taxonomy, apply similarly; otherwise keep as-is.
I can include these tag additions in this PR if you confirm they’re desired.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
config/networks/optimism.json(2 hunks)docs/modules/ROOT/pages/network_configuration.adoc(2 hunks)src/models/network/evm/network.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: test
- GitHub Check: clippy
- GitHub Check: msrv
- GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
- GitHub Check: boostsecurity - boostsecurityio/osv-scanner
- GitHub Check: boostsecurity - boostsecurityio/scanner
- GitHub Check: boostsecurity - boostsecurityio/boost-sca
- GitHub Check: Redirect rules - openzeppelin-relayer
- GitHub Check: Header rules - openzeppelin-relayer
- GitHub Check: Pages changed - openzeppelin-relayer
- GitHub Check: Analyze (rust)
- GitHub Check: semgrep/ci
🔇 Additional comments (3)
docs/modules/ROOT/pages/network_configuration.adoc (1)
326-326: Example updated to the new tag looks good.Sample config now uses "optimism-based" correctly.
config/networks/optimism.json (2)
26-27: Tag updated to "optimism-based" (mainnet): looks correct.This aligns with the new convention and the code change to is_optimism().
53-54: Tag updated to "optimism-based" (sepolia): looks correct.Consistent with mainnet and doc changes.
| |`optimism-based` | ||
| |Identifies Optimism-based networks using the OP Stack (e.g., Optimism, Base, World Chain) | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Rename acknowledged; verify configs for OP Stack networks align with docs.
Docs now define "optimism-based" to include Optimism, Base, and World Chain. In config/networks/optimism.json, Base and World Chain entries currently lack the "optimism-based" tag. If they are intended to be treated as OP Stack in code paths (e.g., fee handling via is_optimism()), consider adding the tag to those entries for consistency.
Run this to check which OP Stack networks in this file are missing the tag:
If intended, update those entries’ tags to include "optimism-based". I can push a follow-up patch if you want.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
jq -r '
.networks[]
| select(.network=="base" or .network=="base-sepolia" or .network=="world-chain" or .network=="world-chain-sepolia")
| "\(.network): " + ( ( .tags // [] | index("optimism-based") ) as $has | if $has then "ok" else "missing" end )
' config/networks/optimism.jsonLength of output: 395
Add optimism-based tag to OP Stack network configs
The docs now categorize Optimism, Base, and World Chain as “optimism-based,” but in config/networks/optimism.json the following entries are missing that tag:
basebase-sepoliaworld-chainworld-chain-sepolia
Please update each of these to include "optimism-based" in its "tags" array. For example:
{
"network": "base",
- "tags": ["mainnet", "layer2"]
+ "tags": ["mainnet", "layer2", "optimism-based"]
},Repeat for the other three entries.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In config/networks/optimism.json (entries "base", "base-sepolia", "world-chain",
"world-chain-sepolia"), add the string "optimism-based" to each network's "tags"
array so they match the docs; ensure you preserve existing tags and JSON
formatting (comma placement) and repeat the same update for all four entries.
| self.tags.contains(&"optimism-based".to_string()) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Switched to "optimism-based": update tests and consider a small refactor.
-
Breaking tests: Several tests in this file still use the "optimism" tag and will now fail because is_optimism() checks for "optimism-based". Please update those fixtures and expectations.
-
Micro-refactor: Avoid allocating a String just to compare.
Apply within this method:
- self.tags.contains(&"optimism-based".to_string())
+ self.tags.iter().any(|t| t == "optimism-based")Follow-up actions outside this hunk:
- Update test vectors and names to the new tag. For example:
- test_is_optimism_with_optimism_tag → test_is_optimism_with_optimism_based_tag
- vec!["optimism", "rollup"] → vec!["optimism-based", "rollup"]
- tags: Some(vec!["rollup".to_string(), "optimism".to_string()]) → tags: Some(vec!["rollup".to_string(), "optimism-based".to_string()])
Use this quick scan to locate remaining occurrences of the old tag:
Optional: to improve consistency and avoid repeated allocations across tag checks, consider a small helper like:
impl EvmNetwork {
fn has_tag(&self, tag: &str) -> bool {
self.tags.iter().any(|t| t == tag)
}
// then use: self.has_tag("optimism-based"), self.has_tag("rollup"), etc.
}🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 --type=rust --type=json --type=adoc '\b"optimism"\b|optimism\)' | sed 's/^/HIT: /'Length of output: 143
The built-in adoc type isn’t recognized by ripgrep—let’s drop it and re-scan only Rust (and JSON) files to find any lingering "optimism" tags:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 --type=rust --type=json '\b"optimism"\b|optimism\)' | sed 's/^/HIT: /'Length of output: 625
Update tests for the “optimism-based” tag and refactor is_optimism
You’ve switched the tag from "optimism" to "optimism-based", so we need to:
• Fix the test in src/domain/transaction/evm/price_calculator.rs
At line 946, change the fixture to use the new tag:
- let optimism = create_mock_evm_network("optimism");
+ let optimism = create_mock_evm_network("optimism-based");• Refactor is_optimism in src/models/network/evm/network.rs
Avoid allocating a String on each check:
impl EvmNetwork {
pub fn is_optimism(&self) -> bool {
- self.tags.contains(&"optimism-based".to_string())
+ self.tags.iter().any(|t| t == "optimism-based")
}
}Optional consistency improvement: add a helper for tag checks:
impl EvmNetwork {
fn has_tag(&self, tag: &str) -> bool {
self.tags.iter().any(|t| t == tag)
}
// then use self.has_tag("optimism-based"), etc.
}Run rg -n --type=rust '\b"optimism"\b' to catch any other occurrences.
🤖 Prompt for AI Agents
In src/models/network/evm/network.rs around lines 96-97 and
src/domain/transaction/evm/price_calculator.rs around line 946, update the test
fixture and refactor the tag check: change the test fixture string from
"optimism" to "optimism-based" at the referenced line in price_calculator.rs,
and modify EvmNetwork::is_optimism to avoid allocating a String on every call by
comparing tags as &str (e.g., use self.tags.iter().any(|t| t ==
"optimism-based") or add a has_tag(&self, tag: &str) helper and call it from
is_optimism). After changes, run rg -n --type=rust '\b"optimism"\b' to find any
remaining occurrences to update.
Summary
Change optimism tag from
optimismtooptimism-basedto align witharbitrum-basedtag.Summary by CodeRabbit