-
Notifications
You must be signed in to change notification settings - Fork 1
Remove arbitrum2 network #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe configuration file was updated to remove the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
settings.yaml(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Siddharth2207
PR: rainlanguage/rain.strategies#42
File: src/fixed-limit.rain:19-23
Timestamp: 2025-06-19T03:59:46.063Z
Learning: In rain.strategies configuration files, `arbitrum2` is an intentional alias for the Arbitrum network (same chain-id 42161) but with updated contract addresses. It serves as a deployment target for newer contract versions while maintaining the existing `arbitrum` configuration during transition periods.
Learnt from: findolor
PR: rainlanguage/rain.strategies#29
File: settings.yaml:1-35
Timestamp: 2025-04-30T06:32:49.466Z
Learning: In the Rain Strategies project, the `matchain` network referenced in `orderbooks` and `deployers` sections of the configuration YAML doesn't require an explicit definition in the `networks` section.
Learnt from: findolor
PR: rainlanguage/rain.strategies#29
File: settings.yaml:2-6
Timestamp: 2025-04-30T06:32:43.860Z
Learning: The `network-id` field is intentionally omitted for the flare network in the `settings.yaml` configuration file, unlike other networks which include both `chain-id` and `network-id` fields.
settings.yaml (3)
Learnt from: Siddharth2207
PR: rainlanguage/rain.strategies#42
File: src/fixed-limit.rain:19-23
Timestamp: 2025-06-19T03:59:46.063Z
Learning: In rain.strategies configuration files, `arbitrum2` is an intentional alias for the Arbitrum network (same chain-id 42161) but with updated contract addresses. It serves as a deployment target for newer contract versions while maintaining the existing `arbitrum` configuration during transition periods.
Learnt from: findolor
PR: rainlanguage/rain.strategies#29
File: settings.yaml:1-35
Timestamp: 2025-04-30T06:32:49.466Z
Learning: In the Rain Strategies project, the `matchain` network referenced in `orderbooks` and `deployers` sections of the configuration YAML doesn't require an explicit definition in the `networks` section.
Learnt from: findolor
PR: rainlanguage/rain.strategies#29
File: settings.yaml:2-6
Timestamp: 2025-04-30T06:32:43.860Z
Learning: The `network-id` field is intentionally omitted for the flare network in the `settings.yaml` configuration file, unlike other networks which include both `chain-id` and `network-id` fields.
🔇 Additional comments (1)
settings.yaml (1)
107-111: No change needed forarbitrum2aliasThe
arbitrum2entry is an intentional, transitional alias pointing to the same Arbitrum chain (ID 42161) with updated contracts. Keepingnetwork: arbitrumis correct for chain consistency—no removal or renaming is required.• Location:
settings.yaml(around lines 107–111)
• Alias purpose: transitional deployment target alongside the existingarbitrumconfigIf clarity is a concern, consider adding a comment above the block explaining that
arbitrum2is an intentional alias for the same network.Likely an incorrect or invalid review comment.
| arbitrum2: | ||
| address: 0xF6FdCB30FD027C73cBacD54E22f02987E3116eBc | ||
| network: arbitrum2 | ||
| network: arbitrum | ||
| subgraph: arbitrum2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
arbitrum2 orderbook now points at arbitrum ― high risk of address / sub-graph mismatch
network: arbitrum no longer aligns with the address (0xF6Fd…) nor with the dedicated sub-graph that still lives under the arbitrum2 key.
If the contracts behind 0xF6Fd… are the new deployments that were deliberately isolated under the arbitrum2 alias (see long-term learning from PR #42), switching the logical network back to arbitrum will cause consumer code to:
- query the arbitrum RPC set while
- using arbitrum2-only addresses and sub-graphs.
That combination is very likely to fail at runtime or, worse, produce silent data corruption.
- network: arbitrum
+ # ⚠️ verify before merging
+ # Either restore the alias…
+ # network: arbitrum2
+ # …or drop this entire `arbitrum2` block if the alias is really gone.Please double-check the deployment history and update the config consistently (networks, orderbooks, deployers and sub-graphs) before merging.
🏁 Script executed:
#!/bin/bash
# Locate the settings.yaml file
file=$(fd settings.yaml | head -n1)
echo "Settings file: $file"
echo "---- Networks section ----"
sed -n '/^networks:/,/^orderbooks:/p' "$file"
echo
echo "---- Orderbooks section (around arbitrum2) ----"
sed -n '/^orderbooks:/,/^deployers:/p' "$file" | sed -n '/arbitrum2:/,/^[^ ]/p'
echo
echo "---- All references to 'arbitrum2' ----"
grep -R -n "arbitrum2" "$file"Length of output: 4090
🏁 Script executed:
#!/bin/bash
file=$(fd settings.yaml | head -n1)
echo "---- Deployers section ----"
sed -n '/^deployers:/,/^[^ ]/p' "$file"Length of output: 865
Ensure arbitrum2 blocks use the correct network alias
Both your orderbooks.arbitrum2 and deployers.arbitrum2 entries are still set to
network: arbitrumbut they reference arbitrum2-only addresses and subgraphs. This will route RPC calls to the legacy Arbitrum endpoint instead of your new deployments. You should either switch these to the arbitrum2 alias or remove the blocks if the alias is deprecated:
• settings.yaml, lines 79–82 (orderbooks.arbitrum2)
• settings.yaml, lines 108–110 (deployers.arbitrum2)
Proposed diff:
orderbooks:
arbitrum2:
address: 0xF6FdCB30FD027C73cBacD54E22f02987E3116eBc
- network: arbitrum
+ network: arbitrum2
subgraph: arbitrum2
deployers:
arbitrum2:
address: 0x9B0D254bd858208074De3d2DaF5af11b3D2F377F
- network: arbitrum
+ network: arbitrum2If you’ve officially deprecated the arbitrum2 alias, remove its orderbook and deployer entries entirely.
🤖 Prompt for AI Agents
In settings.yaml around lines 79 to 82 and 108 to 110, the network alias for
arbitrum2 blocks is incorrectly set to "arbitrum". Update the network field in
both orderbooks.arbitrum2 and deployers.arbitrum2 entries to use the "arbitrum2"
alias instead of "arbitrum". If the arbitrum2 alias is deprecated, remove these
arbitrum2-specific orderbook and deployer entries entirely to avoid routing RPC
calls to the legacy Arbitrum endpoint.
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
arbitrum2network.arbitrumnetwork instead ofarbitrum2.