-
Notifications
You must be signed in to change notification settings - Fork 1
Orderbook V5 settings #45
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
WalkthroughThis change set comments out all network configurations except for Arbitrum across multiple configuration and strategy files, updates Arbitrum-specific addresses and bindings, and adds a new documentation file ( Changes
Sequence Diagram(s)Not applicable: No new features or control flow changes introduced; changes are limited to configuration and documentation. Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsregistry (5)Learnt from: Siddharth2207 Learnt from: findolor Learnt from: Siddharth2207 Learnt from: thedavidmeister Learnt from: thedavidmeister ✨ 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. 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: 4
🔭 Outside diff range comments (1)
src/fixed-limit.rain (1)
208-215: DRY violation: duplicated token-list URLsThe same six
using-tokens-fromURLs appear verbatim across multiple strategy files. Extract them intosettings.yaml(ortoken-lists.yaml) and reference from each strategy to avoid drift.
♻️ Duplicate comments (2)
src/canary.rain (1)
50-64: Checksum & duplication concerns identical to fixed-limitThe new Arbitrum orderbook / deployer / subparser addresses are repeated here with inconsistent casing. Apply the same checksum & centralisation refactor suggested for
src/fixed-limit.rain.Also applies to: 62-64, 120-120
src/grid.rain (1)
58-73: Checksum & duplication concerns identical to fixed-limitSame three addresses appear again; convert to EIP-55 and deduplicate.
Also applies to: 72-73, 127-128
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
CLAUDE.md(1 hunks)registry(1 hunks)settings.yaml(1 hunks)src/auction-dca.rain(2 hunks)src/canary.rain(3 hunks)src/dynamic-spread.rain(3 hunks)src/fixed-limit.rain(3 hunks)src/grid.rain(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 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.
registry (3)
Learnt from: Siddharth2207
PR: #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: thedavidmeister
PR: #28
File: src/folio.rain:28-42
Timestamp: 2025-07-03T09:47:48.019Z
Learning: In Rain language strategy files, thedavidmeister prefers intentionally oversimplified setups over complex validation logic, especially in initial implementations like the folio strategy configuration.
Learnt from: thedavidmeister
PR: #28
File: src/folio.rain:115-122
Timestamp: 2025-07-03T09:47:14.894Z
Learning: In Rain language folio strategy, the zero balance check using every(input-vault-before() output-vault-before()) is specifically implemented to prevent trades from executing while users are depositing or withdrawing from vaults, serving as a safety mechanism against race conditions during vault operations.
CLAUDE.md (3)
Learnt from: thedavidmeister
PR: #28
File: src/folio.rain:28-42
Timestamp: 2025-07-03T09:47:48.019Z
Learning: In Rain language strategy files, thedavidmeister prefers intentionally oversimplified setups over complex validation logic, especially in initial implementations like the folio strategy configuration.
Learnt from: findolor
PR: #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: Siddharth2207
PR: #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.
src/auction-dca.rain (2)
Learnt from: Siddharth2207
PR: #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: #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.
src/dynamic-spread.rain (3)
Learnt from: Siddharth2207
PR: #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: #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: #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.
src/canary.rain (3)
Learnt from: findolor
PR: #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: Siddharth2207
PR: #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: #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: findolor
PR: #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.
Learnt from: Siddharth2207
PR: #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: #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.
src/grid.rain (2)
Learnt from: Siddharth2207
PR: #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: #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.
src/fixed-limit.rain (4)
Learnt from: Siddharth2207
PR: #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: thedavidmeister
PR: #28
File: src/folio.rain:119-119
Timestamp: 2025-07-03T09:46:28.864Z
Learning: In Rain language configuration files, binding values like threshold are compile-time constants, so validation errors (such as division by zero) are caught during compilation rather than requiring runtime checks.
Learnt from: findolor
PR: #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: Siddharth2207
PR: #56
File: settings.yaml:74-74
Timestamp: 2025-07-25T04:36:42.979Z
Learning: In the Rain Strategies project settings.yaml file, Ethereum addresses should use checksum format (EIP-55 with mixed case) rather than all lowercase, as the checksum format provides error detection benefits that outweigh visual consistency concerns.
🪛 LanguageTool
CLAUDE.md
[grammar] ~1-~1: Use correct spacing
Context: # CLAUDE.md This file provides guidance to Claude Co...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~5-~5: Use correct spacing
Context: ...th code in this repository. ## Project Overview Rain Strategies is a collection of tradi...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~7-~7: There might be a mistake here.
Context: ... in the Rain language for decentralized orderbook trading. The repository contains strate...
(QB_NEW_EN_OTHER)
[grammar] ~7-~7: Use correct spacing
Context: ...rading behaviors on multiple blockchain networks. ## Repository Structure - src/: Cont...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~9-~9: Use correct spacing
Context: ...ple blockchain networks. ## Repository Structure - src/: Contains all trading strategy implemen...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~11-~11: Make sure to use plural and singular nouns correctly
Context: ...src/**: Contains all trading strategy implementations - .rain files: Strategy implementations in Rain ...
(QB_NEW_EN_OTHER_ERROR_IDS_10)
[grammar] ~12-~12: There might be a mistake here.
Context: ...files: Strategy implementations in Rain language - .md files: Strategy documentation and explan...
(QB_NEW_EN_OTHER)
[grammar] ~16-~16: Use correct spacing
Context: ...s strategy names to specific Git commit URLs ## Key Trading Strategies The repository i...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~18-~18: Use correct spacing
Context: ...pecific Git commit URLs ## Key Trading Strategies The repository implements several core t...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~20-~20: Use correct spacing
Context: ...ository implements several core trading strategies: 1. Auction DCA (auction-dca.rain): Doll...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~26-~26: Use correct spacing
Context: ...health monitoring strategy that runs on cooldown ## Network Configuration The project suppo...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~28-~28: Use correct spacing
Context: ...ategy that runs on cooldown ## Network Configuration The project supports multiple blockchain...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~31-~31: There might be a mistake here.
Context: ...m** (currently active): Main deployment network - Other networks (Base, BSC, Polygon, Ethe...
(QB_NEW_EN_OTHER)
[grammar] ~32-~32: Insert the missing word
Context: ...networks (Base, BSC, Polygon, Ethereum, Flare, Linea) are commented out Each network ...
(QB_NEW_EN_OTHER_ERROR_IDS_32)
[grammar] ~32-~32: There might be a mistake here.
Context: ..., Ethereum, Flare, Linea) are commented out Each network configuration includes: - R...
(QB_NEW_EN_OTHER)
[grammar] ~40-~40: There might be a mistake here.
Context: ...ndexing - Metaboard URLs for metadata - Orderbook contract addresses - Deployer contract ...
(QB_NEW_EN_OTHER)
[grammar] ~41-~41: Use correct spacing
Context: ... contract addresses - Deployer contract addresses ## Rain Language Specifics Strategy files ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~43-~43: Use correct spacing
Context: ...er contract addresses ## Rain Language Specifics Strategy files use Rain language version...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~49-~49: Use correct spacing
Context: ...arameters - runs: Number of execution cycles ## Development Context This is a strategy ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~51-~51: Use correct spacing
Context: ...ber of execution cycles ## Development Context This is a strategy configuration reposit...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~57-~57: Use correct spacing
Context: ... formatting tools - Package managers or dependencies ## Working with Strategies When modifying ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~59-~59: Use correct spacing
Context: ...nagers or dependencies ## Working with Strategies When modifying strategies: 1. Strategy f...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~62-~62: There might be a mistake here.
Context: ...cific scenarios with different contract addresses 2. Each strategy has accompanying .md doc...
(QB_NEW_EN_OTHER)
[grammar] ~63-~63: There might be a mistake here.
Context: ...d` documentation explaining the trading logic 3. Bindings reference orderbook inputs/outp...
(QB_NEW_EN_OTHER)
[grammar] ~64-~64: There might be a mistake here.
Context: ...the trading logic 3. Bindings reference orderbook inputs/outputs using `${order.inputs.0....
(QB_NEW_EN_OTHER)
[grammar] ~64-~64: There might be a mistake here.
Context: ...using ${order.inputs.0.token.address} syntax 4. Contract addresses (raindex-subparser) v...
(QB_NEW_EN_OTHER)
[grammar] ~65-~65: There might be a mistake here.
Context: ...vary by network and must match deployed contracts ## Network Deployment Currently only Arbit...
(QB_NEW_EN_OTHER)
[grammar] ~67-~67: Use correct spacing
Context: ...st match deployed contracts ## Network Deployment Currently only Arbitrum network is activ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~69-~69: Use articles correctly
Context: ... ## Network Deployment Currently only Arbitrum network is actively configured. Other n...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~69-~69: Use correct spacing
Context: ...ting and updating contract addresses as needed. ## Important Instructions for Claude Code ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~71-~71: Use correct spacing
Context: .... ## Important Instructions for Claude Code NEVER commit changes automatically. Al...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 markdownlint-cli2 (0.17.2)
CLAUDE.md
31-31: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
35-35: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
46-46: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
54-54: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
62-62: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
73-73: Files should end with a single newline character
(MD047, single-trailing-newline)
🔇 Additional comments (7)
src/fixed-limit.rain (1)
70-72: Convert raw hex strings to EIP-55 checksum formatWe computed the correct checksummed variants for the three newly-introduced addresses:
- 0x2f209e5b67a33b8fe96e28f24628df6da301c8eb → 0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB
(Arbitrum entry at lines 70–72 is already correct)- 0x5e9e22154d71f9fd4b5fd1ee36852a4c9fcdcc9e → 0x5E9e22154D71f9Fd4b5FD1eE36852A4C9FcDcC9E
- 0xe80e7438ce6b1055c8e9cde1b6336a4f9d53c666 → 0xe80e7438ce6b1055c8e9CDE1b6336a4F9D53C666
Please apply these replacements at:
- src/fixed-limit.rain, lines 86–88 (Polygon/orderbook address)
- src/fixed-limit.rain, line 146 (subparser address)
@@ src/fixed-limit.rain (lines 86–88) - address: 0x5e9e22154d71f9fd4b5fd1ee36852a4c9fcdcc9e + address: 0x5E9e22154D71f9Fd4b5FD1eE36852A4C9FcDcC9E @@ src/fixed-limit.rain (line 146) - address: 0xe80e7438ce6b1055c8e9cde1b6336a4f9d53c666 + address: 0xe80e7438ce6b1055c8e9CDE1b6336a4F9D53C666We also ran
rg -oI --pcre2 '(0x[a-f0-9]{40})' | sort -uand found several other all-lowercase hex literals. Please review that list and whitelist any values that must remain lowercase (e.g. CREATE2 salts).
src/dynamic-spread.rain (2)
14-73: Configuration consolidation approved.The systematic commenting out of non-Arbitrum network scenarios aligns with the repository-wide focus on Arbitrum deployment. The configurations are preserved for potential future reactivation.
8-8: Contract addresses updated consistently – please confirm on-chain deployment and checksumAll three Arbitrum contract addresses (raindex-subparser, orderbook, deployer) have been updated in lockstep across all strategy files (src/grid.rain, src/fixed-limit.rain, src/dynamic-spread.rain, src/canary.rain, src/auction-dca.rain). Before merging, please:
- Verify each address is in valid EIP-55 checksum format.
- Confirm the corresponding contracts are deployed and verified on the Arbitrum Explorer (under the intended
arbitrum2alias if used).src/auction-dca.rain (2)
10-83: Network consolidation implemented correctly.The systematic commenting out of non-Arbitrum network configurations across all sections (networks, subgraphs, metaboards, orderbooks, deployers, orders, scenarios, deployments) ensures clean Arbitrum-only deployment while preserving configurations for future use.
Also applies to: 100-245
148-149: Arbitrum scenario bindings updated correctly.The raindex-subparser and subparser-0 addresses have been updated to the new contract deployment. The consistent addressing across bindings suggests proper contract coordination.
CLAUDE.md (2)
1-17: Comprehensive project documentation provided.The project overview and repository structure documentation accurately describes the Rain Strategies codebase and provides valuable context for understanding the trading strategy implementations.
28-70: Accurate network deployment documentation.The documentation correctly reflects the current Arbitrum-only deployment strategy and provides comprehensive guidance on network configuration structure and Rain language specifics.
CLAUDE.md
Outdated
| ## Important Instructions for Claude Code | ||
|
|
||
| **NEVER commit changes automatically.** Always wait for explicit user approval before committing any changes to the repository. The user must personally review all changes before they are committed. No newline at end of 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.
🧹 Nitpick (assertive)
Address markdown formatting issues.
The Claude Code instructions are appropriate, but several markdown formatting issues should be resolved for consistency.
Apply these formatting fixes:
+
## Important Instructions for Claude Code
+
**NEVER commit changes automatically.** Always wait for explicit user approval before committing any changes to the repository. The user must personally review all changes before they are committed.
+Additional formatting improvements throughout the file:
- Add blank lines around all list sections (lines 31, 35, 46, 54, 62) per markdown best practices
- The content is excellent but proper spacing improves readability and compliance with markdown standards
📝 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.
| ## Important Instructions for Claude Code | |
| **NEVER commit changes automatically.** Always wait for explicit user approval before committing any changes to the repository. The user must personally review all changes before they are committed. | |
| ## Important Instructions for Claude Code | |
| **NEVER commit changes automatically.** Always wait for explicit user approval before committing any changes to the repository. The user must personally review all changes before they are committed. | |
🧰 Tools
🪛 LanguageTool
[grammar] ~71-~71: Use correct spacing
Context: .... ## Important Instructions for Claude Code NEVER commit changes automatically. Al...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 markdownlint-cli2 (0.17.2)
73-73: Files should end with a single newline character
(MD047, single-trailing-newline)
🤖 Prompt for AI Agents
In CLAUDE.md around lines 31, 35, 46, 54, 62, and the section at lines 71 to 73,
fix markdown formatting by adding blank lines before and after all list sections
to improve readability and comply with markdown standards. Review the entire
file for consistent spacing around headings, lists, and paragraphs, ensuring
proper blank lines separate these elements for better structure and clarity.
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.
this is good enough for instructing claude code
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.
@0xgleb Thank you for the feedback! You're right that the CLAUDE.md file effectively communicates the essential information Claude Code needs to understand the repository structure, Rain language strategies, and the critical instruction to never auto-commit changes. The content successfully covers the key aspects of working with this trading strategy repository.
/| /|
( :v: )
|(_)|
🧠 Learnings used
Learnt from: thedavidmeister
PR: rainlanguage/rain.strategies#28
File: src/folio.rain:28-42
Timestamp: 2025-07-03T09:47:48.019Z
Learning: In Rain language strategy files, thedavidmeister prefers intentionally oversimplified setups over complex validation logic, especially in initial implementations like the folio strategy configuration.
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: 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.
registry
Outdated
| fixed-limit https://raw.githubusercontent.com/rainlanguage/rain.strategies/2342bebc68992a9576950e072819c971b2e7b84c/src/fixed-limit.rain | ||
| auction-dca https://raw.githubusercontent.com/rainlanguage/rain.strategies/2342bebc68992a9576950e072819c971b2e7b84c/src/auction-dca.rain | ||
| grid https://raw.githubusercontent.com/rainlanguage/rain.strategies/2342bebc68992a9576950e072819c971b2e7b84c/src/grid.rain | ||
| dynamic-spread https://raw.githubusercontent.com/rainlanguage/rain.strategies/2342bebc68992a9576950e072819c971b2e7b84c/src/dynamic-spread.rain | ||
| canary https://raw.githubusercontent.com/rainlanguage/rain.strategies/2342bebc68992a9576950e072819c971b2e7b84c/src/canary.rain |
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.
🧹 Nitpick (assertive)
Consider using tags or branches instead of hard-coding commit SHAs
Pinning every strategy reference to a full 40-char commit SHA means any bump requires touching this file, creating noisy diffs and a risk of dangling references.
If immutability isn’t strictly required, reference a protected tag (e.g. orderbook-v5) or branch so that future updates propagate automatically.
🤖 Prompt for AI Agents
In the "registry" file at lines 1 to 5, the strategy URLs are pinned to specific
commit SHAs, causing frequent updates and potential dangling references. To fix
this, replace the full 40-character commit SHAs with references to stable tags
or protected branches, such as "orderbook-v5," so that updates can propagate
automatically without modifying this file each time.
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.
currently using commit SHAs is the pattern. we might change that later
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.
@0xgleb, understood! I'll note that using commit SHAs for version pinning in the registry is the current established pattern for this project.
✏️ Learnings added
Learnt from: 0xgleb
PR: rainlanguage/rain.strategies#45
File: registry:0-0
Timestamp: 2025-07-26T15:31:37.886Z
Learning: In the rain.strategies repository, the registry file uses full commit SHAs to pin strategy URLs as the current established pattern, rather than using tags or branches, as confirmed by project maintainer 0xgleb.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: thedavidmeister
PR: rainlanguage/rain.strategies#28
File: src/folio.rain:28-42
Timestamp: 2025-07-03T09:47:48.019Z
Learning: In Rain language strategy files, thedavidmeister prefers intentionally oversimplified setups over complex validation logic, especially in initial implementations like the folio strategy configuration.
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: thedavidmeister
PR: rainlanguage/rain.strategies#28
File: src/folio.rain:115-122
Timestamp: 2025-07-03T09:47:14.894Z
Learning: In Rain language folio strategy, the zero balance check using `every(input-vault-before() output-vault-before())` is specifically implemented to prevent trades from executing while users are depositing or withdrawing from vaults, serving as a safety mechanism against race conditions during vault operations.
Learnt from: thedavidmeister
PR: rainlanguage/rain.strategies#28
File: src/folio.rain:119-119
Timestamp: 2025-07-03T09:46:28.864Z
Learning: In Rain language configuration files, binding values like `threshold` are compile-time constants, so validation errors (such as division by zero) are caught during compilation rather than requiring runtime checks.
Learnt from: Siddharth2207
PR: rainlanguage/rain.strategies#56
File: settings.yaml:74-74
Timestamp: 2025-07-25T04:36:42.979Z
Learning: In the Rain Strategies project settings.yaml file, Ethereum addresses should use checksum format (EIP-55 with mixed case) rather than all lowercase, as the checksum format provides error detection benefits that outweigh visual consistency concerns.
Learnt from: findolor
PR: rainlanguage/rain.strategies#29
File: settings.yaml:36-44
Timestamp: 2025-04-30T06:33:10.474Z
Learning: URLs in YAML files for the rain.strategies repository don't need to be quoted according to the project maintainer's preference, even though they contain characters like "://" that might be misinterpreted by some YAML parsers.
| arbitrum: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/ob4-arbitrum-one/2025-07-01-8700/gn | ||
| # bsc: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/ob4-bsc/2024-12-13-2244/gn | ||
| # linea: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/ob4-linea/2024-12-13-09c7/gn | ||
| # ethereum: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/ob4-mainnet/2024-12-13-7f22/gn | ||
| metaboards: | ||
| flare: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/mb-flare-0x893BBFB7/0.1/gn | ||
| base: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/metadata-base/2025-07-06-594f/gn | ||
| base2: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/metadata-base/2025-07-06-594f/gn | ||
| polygon: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/mb-polygon/0.1/gn | ||
| # flare: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/mb-flare-0x893BBFB7/0.1/gn | ||
| # base: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/metadata-base/2025-07-06-594f/gn | ||
| # polygon: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/mb-polygon/0.1/gn | ||
| arbitrum: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/metadata-arbitrum-one/2025-07-06-135f/gn | ||
| arbitrum2: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/metadata-arbitrum-one/2025-07-06-135f/gn | ||
| bsc: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/mb-bsc/0.1/gn | ||
| linea: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/mb-linea-0xed7d6156/1.0.0/gn | ||
| ethereum: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/metadata-mainnet/2024-10-25-2857/gn | ||
| # bsc: https://api.goldsky.com/api/public/project_clv14x04y9kzi01saerx7bxpg/subgraphs/mb-bsc/0.1/gn |
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.
🧹 Nitpick (assertive)
Address not checksummed
0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB needs EIP-55 casing here as well.
🤖 Prompt for AI Agents
In settings.yaml around lines 48 to 57, the Ethereum address
`0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB` is not checksummed according to
EIP-55. Update the address to use the correct EIP-55 mixed-case checksum format
to ensure proper validation and consistency.
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.
this shouldn't be an issue
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.
@0xgleb I understand your perspective. I referenced this based on a previous discussion in PR #56 where checksummed addresses were preferred for the settings.yaml file in this project, but I acknowledge you may have different considerations for this specific case.
✏️ Learnings added
Learnt from: 0xgleb
PR: rainlanguage/rain.strategies#45
File: settings.yaml:48-57
Timestamp: 2025-07-26T15:32:06.041Z
Learning: 0xgleb indicated that checksummed addresses are not required to be an issue in settings.yaml for the rain.strategies project, disagreeing with the previous preference for EIP-55 checksum format.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
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.
Learnt from: Siddharth2207
PR: rainlanguage/rain.strategies#56
File: settings.yaml:74-74
Timestamp: 2025-07-25T04:36:42.979Z
Learning: In the Rain Strategies project settings.yaml file, Ethereum addresses should use checksum format (EIP-55 with mixed case) rather than all lowercase, as the checksum format provides error detection benefits that outweigh visual consistency concerns.
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.
| arbitrum: | ||
| address: 0x550878091b2B1506069F61ae59e3A5484Bca9166 | ||
| address: 0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB | ||
| network: arbitrum | ||
| subgraph: arbitrum | ||
| arbitrum2: | ||
| address: 0xF6FdCB30FD027C73cBacD54E22f02987E3116eBc | ||
| network: arbitrum | ||
| subgraph: arbitrum2 | ||
| bsc: | ||
| address: 0xd2938E7c9fe3597F78832CE780Feb61945c377d7 | ||
| network: bsc | ||
| subgraph: bsc | ||
| linea: | ||
| address: 0x22410e2a46261a1B1e3899a072f303022801C764 | ||
| network: linea | ||
| subgraph: linea | ||
| ethereum: | ||
| address: 0x0eA6d458488d1cf51695e1D6e4744e6FB715d37C | ||
| network: ethereum | ||
| subgraph: ethereum | ||
| # bsc: |
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.
🛠️ Refactor suggestion
Single source of truth: let strategy files import from settings.yaml
settings.yaml already maintains the canonical Arbitrum orderbook and deployer addresses.
Rather than embedding the same strings inside every .rain file, make the strategies resolve them via ${settings.orderbooks.arbitrum.address} (or similar include mechanism), eliminating copy errors and easing future upgrades.
Also applies to: 100-103
🤖 Prompt for AI Agents
In settings.yaml around lines 73 to 77 and also lines 100 to 103, the Arbitrum
orderbook and deployer addresses are duplicated in strategy files. To fix this,
remove the hardcoded addresses from the strategy files and update them to import
or reference these values directly from settings.yaml using a variable
substitution or include mechanism like ${settings.orderbooks.arbitrum.address}.
This ensures a single source of truth and prevents copy errors during upgrades.
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.
this is beyond the scope of this PR and/or a conscious design decision made by other contributors
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.
@0xgleb Understood! I appreciate you clarifying the scope and acknowledging that this may be an intentional design choice by the team. I'll focus my reviews on the current changes rather than suggesting broader architectural refactoring.
🐰💭
"Got it - staying
within the lines!"
🧠 Learnings used
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.
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: thedavidmeister
PR: rainlanguage/rain.strategies#28
File: src/folio.rain:28-42
Timestamp: 2025-07-03T09:47:48.019Z
Learning: In Rain language strategy files, thedavidmeister prefers intentionally oversimplified setups over complex validation logic, especially in initial implementations like the folio strategy configuration.
Learnt from: 0xgleb
PR: rainlanguage/rain.strategies#45
File: registry:0-0
Timestamp: 2025-07-26T15:31:37.886Z
Learning: In the rain.strategies repository, the registry file uses full commit SHAs to pin strategy URLs as the current established pattern, rather than using tags or branches, as confirmed by project maintainer 0xgleb.
Learnt from: Siddharth2207
PR: rainlanguage/rain.strategies#56
File: settings.yaml:74-74
Timestamp: 2025-07-25T04:36:42.979Z
Learning: In the Rain Strategies project settings.yaml file, Ethereum addresses should use checksum format (EIP-55 with mixed case) rather than all lowercase, as the checksum format provides error detection benefits that outweigh visual consistency concerns.
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: 0xgleb
PR: rainlanguage/rain.strategies#45
File: settings.yaml:48-57
Timestamp: 2025-07-26T15:32:06.041Z
Learning: 0xgleb indicated that checksummed addresses are not required to be an issue in settings.yaml for the rain.strategies project, disagreeing with the previous preference for EIP-55 checksum format.
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
Documentation
Chores