-
Notifications
You must be signed in to change notification settings - Fork 1
Update settings and strategies for registry implementation #70
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughUpdates strategy registry base URL from commit 8fdd42b to 82b1f42, expands settings.yaml to enable multiple networks (BSC, Linea, Ethereum) with deployment metadata, and restructures strategy configuration files by removing top-level infrastructure definitions while preserving core order and GUI specifications. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/canary.rain (1)
10-17: Consider removing or documenting commented network configurations.There are extensive commented-out sections for other networks (base, flare, polygon, bsc). If these are templates for future use, consider moving them to a separate template file or adding a comment explaining their purpose. If they're no longer needed, removing them would improve readability.
src/dynamic-spread.rain (1)
153-155: Broken deployment reference:flarescenario is commented out.The
flaredeployment (lines 153-155) referencesscenario: flare, but theflarescenario at lines 24-33 is commented out. This will cause a configuration error when trying to use this deployment.Either uncomment the flare scenario or comment out/remove the flare deployment:
- flare: - order: flare - scenario: flare + # flare: + # order: flare + # scenario: flareAlternatively, if the flare deployment should work, uncomment the scenario at lines 24-33.
♻️ Duplicate comments (1)
src/dynamic-spread.rain (1)
336-416: GUI Flare deployment depends on the broken flare deployment.This GUI configuration will fail if the underlying
flaredeployment issue (scenario reference) is not resolved.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
registry(1 hunks)settings.yaml(3 hunks)src/auction-dca.rain(0 hunks)src/canary.rain(6 hunks)src/claims.rain(0 hunks)src/dynamic-spread.rain(3 hunks)src/fixed-limit.rain(5 hunks)src/fixed-spread.rain(0 hunks)src/folio.rain(0 hunks)src/grid.rain(0 hunks)
💤 Files with no reviewable changes (5)
- src/auction-dca.rain
- src/claims.rain
- src/grid.rain
- src/folio.rain
- src/fixed-spread.rain
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: 0xgleb
Repo: rainlanguage/rain.strategies PR: 45
File: registry:0-0
Timestamp: 2025-07-26T15:31:37.916Z
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: findolor
Repo: rainlanguage/rain.strategies PR: 59
File: src/fixed-limit.rain:68-68
Timestamp: 2025-09-01T12:19:42.343Z
Learning: In the rain.strategies repository, the team plans to replace contract addresses in the configuration files, and deployment-block values will be updated as part of that broader effort, so placeholder values like 1 are intentionally temporary.
Learnt from: Siddharth2207
Repo: rainlanguage/rain.strategies 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.
📚 Learning: 2025-07-26T15:31:37.916Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.strategies PR: 45
File: registry:0-0
Timestamp: 2025-07-26T15:31:37.916Z
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.
Applied to files:
registry
📚 Learning: 2025-09-01T12:19:42.343Z
Learnt from: findolor
Repo: rainlanguage/rain.strategies PR: 59
File: src/fixed-limit.rain:68-68
Timestamp: 2025-09-01T12:19:42.343Z
Learning: In the rain.strategies repository, the team plans to replace contract addresses in the configuration files, and deployment-block values will be updated as part of that broader effort, so placeholder values like 1 are intentionally temporary.
Applied to files:
registrysrc/fixed-limit.rainsrc/dynamic-spread.rainsrc/canary.rain
📚 Learning: 2025-06-19T03:59:46.063Z
Learnt from: Siddharth2207
Repo: rainlanguage/rain.strategies 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.
Applied to files:
registrysrc/fixed-limit.rainsettings.yamlsrc/dynamic-spread.rainsrc/canary.rain
📚 Learning: 2025-07-03T09:47:48.019Z
Learnt from: thedavidmeister
Repo: rainlanguage/rain.strategies 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.
Applied to files:
registry
📚 Learning: 2025-09-04T12:59:34.219Z
Learnt from: thedavidmeister
Repo: rainlanguage/rain.strategies PR: 60
File: src/dynamic-spread.rain:924-929
Timestamp: 2025-09-04T12:59:34.219Z
Learning: In the Rain Strategies dynamic spread system, the io values cannot be zero due to business logic constraints where baseline uses any(cost-basis-io last-io) and cost-basis-multiplier is positive, making division-by-zero in inv(stored-last-io) mathematically impossible under normal operation.
Applied to files:
registrysrc/dynamic-spread.rain
📚 Learning: 2025-07-03T09:47:14.894Z
Learnt from: thedavidmeister
Repo: rainlanguage/rain.strategies 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.
Applied to files:
registry
📚 Learning: 2025-04-30T06:32:49.466Z
Learnt from: findolor
Repo: rainlanguage/rain.strategies 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.
Applied to files:
src/canary.rain
🔇 Additional comments (8)
settings.yaml (3)
26-43: Network configurations look correct.Chain IDs are accurate (BSC: 56, Linea: 59144, Ethereum: 1), and each network includes the required fields (rpcs, chain-id, network-id, currency).
117-125: Verify duplicate deployer address for BSC and Linea.Both
bsc(line 118) andlinea(line 121) use the same deployer address0xA2f56F8F74B7d04d61f281BE6576b6155581dcBA. This could be intentional if the same contract was deployed to both chains, but please confirm this is correct.
127-133: Token lists configuration looks good.CoinGecko token lists cover the major networks, and the Flare token list is appropriately pinned to a specific commit SHA.
src/canary.rain (1)
3-9: Canary-arbitrum configuration is consistent.The renaming from
arbitrumtocanary-arbitrumallows this strategy to use a distinct orderbook address (0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB) while still referencing thearbitrumnetwork and subgraph from settings.yaml. This pattern enables strategy-specific deployments.src/dynamic-spread.rain (1)
75-86: Strategy-specific Flare configuration looks correct.The
dynamic-spread-flareorderbook and deployer correctly referencenetwork: flareandsubgraph: flarefrom settings.yaml while using a distinct orderbook address for this strategy.src/fixed-limit.rain (2)
3-14: Fixed-limit-arbitrum configuration is consistent.The renaming from
arbitrumtofixed-limit-arbitrumfollows the same pattern as other strategy files. The orderbook and deployer addresses match those incanary.rain, which is expected as these strategies share the same underlying contracts on Arbitrum.
29-34: All fixed-limit-arbitrum references are properly linked.The order, scenario, deployment, and GUI configuration entries are all correctly cross-referenced, ensuring the deployment will work as expected.
Also applies to: 61-65, 111-113, 212-227
registry (1)
1-9: Registry entries verified and correctly pinned.All entries use commit SHA
82b1f42635ebfd8922a269eb422bd6aafee52910with consistent formatting. The commit is valid and all referenced files (settings.yaml and 8 strategy files) exist and are accessible at this commit. This follows the established pattern of pinning strategy URLs with full commit SHAs.
| local-db-remotes: | ||
| raindex: https://example.com/subgraph |
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.
Placeholder URL in local-db-remotes.
The raindex remote points to https://example.com/subgraph, which appears to be a placeholder. Ensure this is updated to a valid endpoint before deployment, or confirm if this is intentionally temporary.
🤖 Prompt for AI Agents
In settings.yaml around lines 135-136 the local-db-remotes.raindex entry uses
the placeholder URL "https://example.com/subgraph"; replace this with the actual
raindex endpoint (or a configuration reference/env var) before deployment, or
explicitly document/annotate that it is intentionally temporary for local
testing and should not be used in production so reviewers know it is safe to
keep as-is.
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.
yeah this is fine for now
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.
@findolor, understood! I've noted that the placeholder is acceptable for the current stage.
🧠 Learnings used
Learnt from: findolor
Repo: rainlanguage/rain.strategies PR: 59
File: src/fixed-limit.rain:68-68
Timestamp: 2025-09-01T12:19:42.343Z
Learning: In the rain.strategies repository, the team plans to replace contract addresses in the configuration files, and deployment-block values will be updated as part of that broader effort, so placeholder values like 1 are intentionally temporary.
Learnt from: findolor
Repo: rainlanguage/rain.strategies 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
Repo: rainlanguage/rain.strategies 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: Siddharth2207
Repo: rainlanguage/rain.strategies 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.
Learnt from: 0xgleb
Repo: rainlanguage/rain.strategies PR: 45
File: registry:0-0
Timestamp: 2025-07-26T15:31:37.916Z
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.
Motivation
With the new register implementation we can now merge settings and the frontmatter in strategies in our SDK. This allows us to split the content in multiple files.
We should be now using the orderbook related settings inside
settings.yamland dotrain related settings inside frontmatter.Solution
settings.yamlif they reference the same valuesChecks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.