Skip to content

Conversation

@Siddharth2207
Copy link
Contributor

@Siddharth2207 Siddharth2207 commented Jul 14, 2025

Motivation

Order to distribute rewards by verifying merkle proofs against a given root

Checks

By submitting this for review, I'm confirming I've done the following:

  • made this PR as small as possible
  • unit-tested any new functionality
  • linked any relevant issues or PRs
  • included screenshots (if this involves a front-end change)

Summary by CodeRabbit

  • New Features
    • Added claims verification and distribution flow using Merkle-proof-based claims.
    • UI-facing Claims interface for submitting proofs and receiving tokens.
    • Multi-network support (Base, Polygon) with network and token selection.
    • Enforced unique-claim validation to prevent duplicate claims and mark claims completed.
    • Added registry entry linking the Claims feature to the app.

@coderabbitai
Copy link

coderabbitai bot commented Jul 14, 2025

Walkthrough

Added a new registry entry "claims" and a new file src/claims.rain implementing multi-network Merkle-proof claim verification and distribution with network configs, UI metadata, deployments, and named logic blocks for leaf construction, claim checking, proof verification, IO calculation, and claim handling. (38 words)

Changes

Cohort / File(s) Change Summary
Registry update
registry
Added a new claims registry entry referencing the new module (points to src/claims.rain).
New claims module
src/claims.rain
Added src/claims.rain implementing network configurations (base, polygon), subgraphs, deployments, UI metadata, and named logic blocks: #get-leaf-node, #check-claim, #verify-proof, #calculate-io, #handle-io, #handle-add-order.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ClaimsModule
    participant Storage
    participant MerkleVerifier

    User->>ClaimsModule: submit claim (orderHash, leaf index, amount, root, proof, context)
    ClaimsModule->>Storage: read(hash(orderHash, leafNode, claimed-key))
    Storage-->>ClaimsModule: 0 / 1
    alt not claimed (0)
        ClaimsModule->>MerkleVerifier: verify(root, leafNode, proof, context)
        MerkleVerifier-->>ClaimsModule: valid / invalid
        alt valid
            ClaimsModule->>Storage: write(hash(orderHash, leafNode, claimed-key) = 1)
            ClaimsModule-->>User: success (amount credited)
        else invalid
            ClaimsModule-->>User: reject (invalid proof)
        end
    else already claimed (1)
        ClaimsModule-->>User: reject (already claimed)
    end
    note right of ClaimsModule #D3E4CD: Phases: get-leaf-node → check-claim → verify-proof → mark-claimed
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 2025-07-14-claims

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 71ab7f4 and e06e45c.

📒 Files selected for processing (2)
  • registry (1 hunks)
  • src/claims.rain (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
registry (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: 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: 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.
src/claims.rain (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: 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.
🔇 Additional comments (4)
registry (1)

6-6: Verify the different commit hash for the claims entry.

The claims entry uses a different commit hash (c29e7cf) compared to all other entries (ae0065c). Please confirm this is intentional and won't cause any compatibility issues with the other strategies.

src/claims.rain (3)

105-112: Clarify the input/output logic discrepancy.

The comment on line 30 states "Input is irrelevant because we always set the IO ratio to 0", but the calculate-io function returns expected-amount as input and 0 as output. This seems contradictory - if input is irrelevant, why is the expected claim amount being returned as input?

Consider either:

  1. Updating the comment to accurately reflect the logic
  2. Swapping the return values if output should receive the claimed amount

109-109: Verify the double hashing pattern for leaf construction.

The leaf is constructed using double hashing: hash(hash(order-counterparty() expected-amount)). While this might be intentional for security reasons, it's worth confirming this matches the expected leaf format for your merkle tree construction.


87-99: Validate Signed Context Origins

It looks like this strategy pulls in nine signed-context<0 N>() values (indices 0 through 8) and feeds them directly into merkle-proof-verify without any DSL-level check of who produced those contexts or what payload they carry. We need to ensure that each context was genuinely signed by an authorized party before trusting it in the proof.

Please verify or implement one of the following:

  • Confirm that the Rain VM’s built-in signed-context opcode automatically enforces ECDSA signature checks against a known keyset.
  • If not, add an explicit signature‐verification step (e.g. with ecdsa-verify or a similar built-in) to guard each signed-context<0 N>() against an approved signer identity.
  • Document the expected signer keys and threat model for these contexts in src/claims.rain (or in adjacent strategy docs) so future reviewers can see how authenticity is guaranteed.

Affected location:
• src/claims.rain lines 87–99 (the nine signed-context<0 1>()signed-context<0 8>() parameters to merkle-proof-verify)

@Siddharth2207 Siddharth2207 self-assigned this Jul 14, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ffa275 and 40c2920.

📒 Files selected for processing (2)
  • registry (1 hunks)
  • src/claims.rain (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
registry (3)
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: 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: 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.
src/claims.rain (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: 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.
🔇 Additional comments (2)
registry (1)

6-6: LGTM!

The registry entry follows the established pattern and correctly references the new claims module.

src/claims.rain (1)

70-117: Well-structured claims verification implementation!

The merkle proof verification logic is correctly implemented with proper security checks:

  • Duplicate claim prevention via the hash-based storage
  • Standard merkle proof verification pattern
  • Clear separation of concerns between verification and state management

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40c2920 and baee121.

📒 Files selected for processing (1)
  • src/claims.rain (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/claims.rain (4)
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: 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: 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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
src/claims.rain (2)

128-129: Critical: Fix the reversed IO ratio in calculate-io.

The function returns (expected-amount, 0) which would require input tokens from claimants but provide no output. For a claims distribution system, this should be (0, expected-amount) - requiring no input and providing the claimed amount as output.

Apply this fix:

-  _: expected-amount,
-  _: 0;
+  _: 0,
+  _: expected-amount;

125-133: Consider optimizing duplicate leaf hash calculation.

Both calculate-io and handle-io compute the same leaf-node hash, which is computationally inefficient. While this ensures consistency, consider storing the leaf hash in a context variable or restructuring to compute it once.

If the duplication is intentional for security/consistency reasons, consider adding a comment explaining this design choice.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between baee121 and 967fcb9.

📒 Files selected for processing (1)
  • src/claims.rain (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/claims.rain (4)
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: 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: 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.
🔇 Additional comments (1)
src/claims.rain (1)

143-144: LGTM: Appropriate no-op implementation.

The handle-add-order function correctly implements a no-op for this claims strategy, as no special handling is needed when orders are added.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 967fcb9 and e926fb3.

📒 Files selected for processing (2)
  • registry (1 hunks)
  • src/claims.rain (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
registry (3)
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: 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: 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.
src/claims.rain (5)
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: 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: 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: Siddharth2207
PR: rainlanguage/rain.strategies#53
File: src/claims.rain:0-0
Timestamp: 2025-07-15T09:19:34.830Z
Learning: In Rain language, the evaluation uses a stack-based model where stack indexes start from the bottom. In the calculate-io function, stack[0] represents the ratio and stack[1] represents the amount. This means when a function returns two values like `(expected-amount, 0)`, the first value is the ratio and the second value is the amount.
🔇 Additional comments (1)
registry (1)

6-6: LGTM!

The claims strategy entry follows the standard registry format and correctly references the implementation file.

Comment on lines +70 to +73
#raindex-subparser !The subparser to use.
#merkle-words !The subparser for merkle words.
#root !Order Root
#claimed-key "claimed"
Copy link

@coderabbitai coderabbitai bot Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Validate that the merkle root is non-zero.

The merkle root should be validated to ensure it's not zero, as a zero root could potentially compromise the proof verification.

Consider adding validation when the order is created to ensure the root is non-zero:

+#validate-root
+  :ensure(
+    greater-than(root 0)
+    "Invalid root"
+  );

Then call this validation in #handle-add-order:

 #handle-add-order
-  :;
+  :call<'validate-root>();
📝 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.

Suggested change
#raindex-subparser !The subparser to use.
#merkle-words !The subparser for merkle words.
#root !Order Root
#claimed-key "claimed"
#raindex-subparser !The subparser to use.
#merkle-words !The subparser for merkle words.
#root !Order Root
#claimed-key "claimed"
#validate-root
:ensure(
greater-than(root 0)
"Invalid root"
);
#handle-add-order
:call<'validate-root'>();
🤖 Prompt for AI Agents
In src/claims.rain around lines 70 to 73, the merkle root currently lacks
validation to ensure it is non-zero, which is critical for proof verification
integrity. Add a validation step when creating the order to check that the root
value is not zero, and integrate this validation call within the
#handle-add-order handler to enforce this constraint before processing the order
further.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Siddharth2207 Understood, I'll skip the merkle root validation. Thanks for the clarification!


🧠 Learnings used
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.

@coderabbitai coderabbitai bot mentioned this pull request Jul 23, 2025
4 tasks
Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between e926fb3 and 5f38d41.

📒 Files selected for processing (2)
  • registry (1 hunks)
  • src/claims.rain (1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: in the rain.strategies repository, the registry file uses full commit shas to pin strategy urls as t...
Learnt from: 0xgleb
PR: rainlanguage/rain.strategies#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
  • src/claims.rain
📚 Learning: in rain language strategy files, thedavidmeister prefers intentionally oversimplified setups over co...
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.

Applied to files:

  • registry
  • src/claims.rain
📚 Learning: in rain language folio strategy, the zero balance check using `every(input-vault-before() output-vau...
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.

Applied to files:

  • registry
  • src/claims.rain
📚 Learning: in rain.strategies configuration files, `arbitrum2` is an intentional alias for the arbitrum network...
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.

Applied to files:

  • registry
  • src/claims.rain
📚 Learning: in the rain strategies project, the `matchain` network referenced in `orderbooks` and `deployers` se...
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.

Applied to files:

  • src/claims.rain
📚 Learning: in the rain strategies project settings.yaml file, ethereum addresses should use checksum format (ei...
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.

Applied to files:

  • src/claims.rain
📚 Learning: in rain language, the evaluation uses a stack-based model where stack indexes start from the bottom....
Learnt from: Siddharth2207
PR: rainlanguage/rain.strategies#53
File: src/claims.rain:0-0
Timestamp: 2025-07-15T09:19:34.859Z
Learning: In Rain language, the evaluation uses a stack-based model where stack indexes start from the bottom. In the calculate-io function, stack[0] represents the ratio and stack[1] represents the amount. This means when a function returns two values like `(expected-amount, 0)`, the first value is the ratio and the second value is the amount.

Applied to files:

  • src/claims.rain
📚 Learning: 0xgleb indicated that checksummed addresses are not required to be an issue in settings.yaml for the...
Learnt from: 0xgleb
PR: rainlanguage/rain.strategies#45
File: settings.yaml:48-57
Timestamp: 2025-07-26T15:32:06.061Z
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.

Applied to files:

  • src/claims.rain
🔇 Additional comments (2)
registry (1)

6-6: LGTM! Registry entry follows established pattern.

The new claims entry correctly uses a full commit SHA to pin the strategy URL, which aligns with the established pattern in this repository.

src/claims.rain (1)

61-62: Use checksummed addresses for binding contracts

The binding contract addresses should also use EIP-55 checksum format for consistency and error detection.

Convert the binding addresses to checksum format:

 scenarios:
   base:
     orderbook: base
     runs: 1
     bindings:
-      raindex-subparser: 0x662dFd6d5B6DF94E07A60954901D3001c24F856a
-      merkle-words: 0x8ceC9e3Ec2F8838000b91CfB97403A6Bb0F4036A
+      raindex-subparser: 0x662dFd6d5B6DF94E07A60954901D3001c24F856a
+      merkle-words: 0x8ceC9e3Ec2F8838000b91CfB97403A6Bb0F4036A
   polygon:
     orderbook: polygon
     runs: 1
     bindings:
-      raindex-subparser: 0xF9323B7d23c655122Fb0272D989b83E105cBcf9d
-      merkle-words: 0x5F0D7f1995504eE43F02b9D13ce3a19cd5Bdd9B0
+      raindex-subparser: 0xF9323B7d23c655122Fb0272D989b83E105cBcf9d
+      merkle-words: 0x5F0D7f1995504eE43F02b9D13ce3a19cd5Bdd9B0

Also applies to: 67-68

⛔ Skipped due to learnings
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: 0xgleb
PR: rainlanguage/rain.strategies#45
File: settings.yaml:48-57
Timestamp: 2025-07-26T15:32:06.061Z
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.

Comment on lines +28 to +30
address: 0xd2938e7c9fe3597f78832ce780feb61945c377d7
polygon:
address: 0x7D2f700b1f6FD75734824EA4578960747bdF269A
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use checksummed addresses for error detection

The contract addresses should use EIP-55 checksum format (mixed case) for error detection. This helps prevent typos and ensures address validity.

Convert the addresses to checksum format:

 orderbooks:
   base:
-    address: 0xd2938e7c9fe3597f78832ce780feb61945c377d7
+    address: 0xD2938E7c9fE3597f78832Ce780fEB61945c377D7
   polygon:
-    address: 0x7D2f700b1f6FD75734824EA4578960747bdF269A
+    address: 0x7D2f700b1f6FD75734824EA4578960747BdF269A

 deployers:
   base:
-    address: 0xC1A14cE2fd58A3A2f99deCb8eDd866204eE07f8D
+    address: 0xC1A14cE2fd58A3A2f99deCb8eDd866204eE07f8D
   polygon:
-    address: 0xE7116BC05C8afe25e5B54b813A74F916B5D42aB1
+    address: 0xE7116BC05C8AFe25e5B54b813A74F916B5D42aB1

Also applies to: 34-36

🤖 Prompt for AI Agents
In src/claims.rain around lines 28 to 30 and also lines 34 to 36, the contract
addresses are not in EIP-55 checksum format, which is important for error
detection and preventing typos. Update these addresses to use the mixed-case
checksum format by converting each address accordingly before committing the
changes.

Copy link

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (3)
src/claims.rain (3)

17-20: Placeholder subgraph endpoints acknowledged

These are placeholders; leaving as-is is fine per prior thread. Tagging as prior feedback to avoid churn.


43-45: Clarify the “input is irrelevant” comment

Comment conflicts with the IO pair usage elsewhere; suggest clarifying it refers to runtime flows, not structural requirements. Marking as prior feedback.

Also applies to: 51-52


26-36: Confirm orderbook/deployer addresses; optionally standardize casing

  • Verify these Base/Polygon orderbook and deployer addresses match your deployments.
  • Optionally convert all entries to EIP-55 checksum format for consistency (project convention).
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 5f38d41 and 1e8cd30.

📒 Files selected for processing (2)
  • registry (1 hunks)
  • src/claims.rain (1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-07-26T15:31:37.916Z
Learnt from: 0xgleb
PR: rainlanguage/rain.strategies#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-07-03T09:47:48.019Z
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.

Applied to files:

  • registry
📚 Learning: 2025-09-01T12:19:42.343Z
Learnt from: findolor
PR: rainlanguage/rain.strategies#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:

  • src/claims.rain
📚 Learning: 2025-07-03T09:47:14.894Z
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.

Applied to files:

  • src/claims.rain
📚 Learning: 2025-06-19T03:59:46.063Z
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.

Applied to files:

  • src/claims.rain
📚 Learning: 2025-07-15T09:19:34.859Z
Learnt from: Siddharth2207
PR: rainlanguage/rain.strategies#53
File: src/claims.rain:0-0
Timestamp: 2025-07-15T09:19:34.859Z
Learning: In Rain language, the evaluation uses a stack-based model where stack indexes start from the bottom. In the calculate-io function, stack[0] represents the ratio and stack[1] represents the amount. This means when a function returns two values like `(expected-amount, 0)`, the first value is the ratio and the second value is the amount.

Applied to files:

  • src/claims.rain
📚 Learning: 2025-07-25T04:36:42.979Z
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.

Applied to files:

  • src/claims.rain
📚 Learning: 2025-07-26T15:32:06.061Z
Learnt from: 0xgleb
PR: rainlanguage/rain.strategies#45
File: settings.yaml:48-57
Timestamp: 2025-07-26T15:32:06.061Z
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.

Applied to files:

  • src/claims.rain
🔇 Additional comments (4)
registry (2)

6-6: Good: entry pinned to full commit SHA

Using a full SHA for claims aligns with the established registry pattern noted by 0xgleb.


6-6: Raw URL verification passed: the URL returns HTTP 200 and correctly points to src/claims.rain at commit 3fcf773.

src/claims.rain (2)

38-55: Ensure token keys resolve

Confirm how tokens input/output are bound (e.g., via gui select-tokens or a tokens map). If CLI or non-GUI flows are used, ensure bindings exist for both networks.


56-69: Validate subparser bindings per network

Double-check the raindex-subparser and merkle-words addresses are deployed on the specified chains and match the expected bytecode/version.

Comment on lines +119 to +130
/* Compute hash of leaf node */
#get-leaf-node
/**
* signed-context<0 0> is the leaf index
* order-counterparty() is the order taker who is claiming the reward
* signed-context<0 1> is the expected claim amount
*
* All the leaves of the merkle tree are computed by precisely these,
* hence the order taker MUST provide the index and the expected amount in the signed context.
*/
_: hash(signed-context<0 0>() order-counterparty() signed-context<0 1>());

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add domain separation to leaf hash

Prefixing the leaf hash with a constant reduces cross-context collision risk and future-proofs the encoding.

 #get-leaf-node
+  leaf-domain: "CLAIM_LEAF_V1",
   /**
@@
-  _: hash(signed-context<0 0>() order-counterparty() signed-context<0 1>());
+  _: hash(leaf-domain signed-context<0 0>() order-counterparty() signed-context<0 1>());
📝 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.

Suggested change
/* Compute hash of leaf node */
#get-leaf-node
/**
* signed-context<0 0> is the leaf index
* order-counterparty() is the order taker who is claiming the reward
* signed-context<0 1> is the expected claim amount
*
* All the leaves of the merkle tree are computed by precisely these,
* hence the order taker MUST provide the index and the expected amount in the signed context.
*/
_: hash(signed-context<0 0>() order-counterparty() signed-context<0 1>());
/* Compute hash of leaf node */
#get-leaf-node
leaf-domain: "CLAIM_LEAF_V1",
/**
* signed-context<0 0> is the leaf index
* order-counterparty() is the order taker who is claiming the reward
* signed-context<0 1> is the expected claim amount
*
* All the leaves of the merkle tree are computed by precisely these,
* hence the order taker MUST provide the index and the expected amount in the signed context.
*/
_: hash(leaf-domain signed-context<0 0>() order-counterparty() signed-context<0 1>());
🤖 Prompt for AI Agents
In src/claims.rain around lines 119 to 130, the leaf hash lacks domain
separation; update the hash input to prefix a constant domain separator (e.g. a
fixed tag like "CLAIM_LEAF_v1" or a short hex constant) before the existing
fields so the leaf is computed as hash(domain-sep, signed-context<0 0>(),
order-counterparty(), signed-context<0 1>()); ensure the exact same domain
separator is used wherever leaves are recomputed/verified (tests, merkle proof
creation/verification) and keep the separator constant and well-documented.

Comment on lines +142 to +164
#verify-proof
leaf-node: ,
/**
* Verifies merkle proof using signed context parameters:
* signed-context<0 2-9>: Merkle proof components for merkle tree with depth 8
*/
is-valid-proof: merkle-proof-verify(
root
leaf-node
signed-context<0 2>()
signed-context<0 3>()
signed-context<0 4>()
signed-context<0 5>()
signed-context<0 6>()
signed-context<0 7>()
signed-context<0 8>()
signed-context<0 9>()
),
:ensure(
is-valid-proof
"Proof"
);

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Depth-8 proof: confirm fits expected max tree size

Hardcoding eight siblings caps the tree at 256 leaves. If larger drops are possible, consider supporting variable depth (e.g., additional signed-context words or multiple verify calls).

🤖 Prompt for AI Agents
In src/claims.rain around lines 142 to 164, the merkle-proof-verify call
hardcodes eight sibling parameters (depth-8) which limits the tree to 256
leaves; update the verification logic to accept variable-depth proofs by reading
a depth parameter (or count of signed-context words) and passing the appropriate
number of signed-context siblings to merkle-proof-verify (or iteratively
verify/aggregate for larger trees), and add input validation to reject proofs
that exceed the allowed maximum depth to avoid silent failures.

Comment on lines 175 to 189
#handle-io
expected-amount: signed-context<0 1>(),
leaf-node: call<'get-leaf-node>(),
:ensure(
equal-to(
output-vault-decrease()
scale-18<output-token-decimals()>(
scale-n<output-token-decimals()>(
expected-amount
)
)
)
"Expected claim"
),
:set(hash(order-hash() leaf-node claimed-key) 1);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Also assert no input was provided

Strengthen invariants: ensure claimants don’t send any input tokens (accidentally or maliciously).

 #handle-io
   expected-amount: signed-context<0 1>(),
   leaf-node: call<'get-leaf-node>(),
+  :ensure(
+      equal-to(input-vault-increase() 0)
+      "No input allowed for claims"
+  ),
   :ensure(
       equal-to(
         output-vault-decrease()
         scale-18<output-token-decimals()>(
           scale-n<output-token-decimals()>(
             expected-amount
           )
         )
       )
       "Expected claim"
   ),
   :set(hash(order-hash() leaf-node claimed-key) 1);
📝 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.

Suggested change
#handle-io
expected-amount: signed-context<0 1>(),
leaf-node: call<'get-leaf-node>(),
:ensure(
equal-to(
output-vault-decrease()
scale-18<output-token-decimals()>(
scale-n<output-token-decimals()>(
expected-amount
)
)
)
"Expected claim"
),
:set(hash(order-hash() leaf-node claimed-key) 1);
#handle-io
expected-amount: signed-context<0 1>(),
leaf-node: call<'get-leaf-node>(),
:ensure(
equal-to(input-vault-increase() 0)
"No input allowed for claims"
),
:ensure(
equal-to(
output-vault-decrease()
scale-18<output-token-decimals()>(
scale-n<output-token-decimals()>(
expected-amount
)
)
)
"Expected claim"
),
:set(hash(order-hash() leaf-node claimed-key) 1);
🤖 Prompt for AI Agents
In src/claims.rain around lines 175–189, add an invariant to ensure no input
tokens are provided by the claimant: insert an :ensure before the :set(...) that
asserts the input vault change is zero (compare input-vault-increase() to 0,
applying the same scale-to-decimals pattern used for output checks — e.g.,
scale-18<input-token-decimals()>(0) or equivalent helper) so any attempt to send
input tokens fails the claim.

Copy link

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (8)
src/claims.rain (8)

17-20: Placeholder subgraph URLs.

Both network subgraph endpoints are placeholders; confirm/update before any production use.


119-130: Add domain separation to the leaf hash.

Prepend a constant tag to the leaf preimage to prevent cross-context collisions and future-proof upgrades.

-#get-leaf-node
+#get-leaf-node
+  leaf-domain: "CLAIM_LEAF_V1",
@@
-  _: hash(signed-context<0 0>() order-counterparty() signed-context<0 1>());
+  _: hash(leaf-domain signed-context<0 0>() order-counterparty() signed-context<0 1>());

131-139: Error message is terse.

Consider “Already claimed” for clarity during support/debugging.


142-164: Error message is terse.

Consider “Invalid proof” to make failures self-explanatory.


175-189: Also assert no input provided.

Disallow accidental/malicious input tokens on claims.

 #handle-io
   expected-amount: signed-context<0 1>(),
   leaf-node: call<'get-leaf-node>(),
+  :ensure(
+    equal-to(input-vault-increase() 0)
+    "No input allowed for claims"
+  ),
   output-decimals: output-token-decimals(),

165-166: Document IO pair semantics.

Add a short note that returns are (ratio, amount) and why ratio=expected-amount with amount=0 is intentional here.


142-159: Depth-8 proof is a hard cap.

If >256 leaves may be required later, consider variable-depth proofs or a bounded-but-configurable depth.


26-36: Addresses: checksum and correctness pass.

  • base.orderbooks.address is lowercase; polygon.deployers appears non-checksummed. If the project prefers EIP-55, apply below; otherwise at least verify the addresses are correct on their networks.
 orderbooks:
   base:
-    address: 0xd2938e7c9fe3597f78832ce780feb61945c377d7
+    address: 0xD2938E7c9fE3597f78832Ce780fEB61945c377D7
 ...
   polygon:
-    address: 0xE7116BC05C8afe25e5B54b813A74F916B5D42aB1
+    address: 0xE7116BC05C8AFe25e5B54b813A74F916B5D42aB1
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 1e8cd30 and 90ce3f1.

📒 Files selected for processing (2)
  • registry (1 hunks)
  • src/claims.rain (1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-07-26T15:31:37.916Z
Learnt from: 0xgleb
PR: rainlanguage/rain.strategies#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-07-03T09:47:48.019Z
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.

Applied to files:

  • registry
📚 Learning: 2025-09-01T12:19:42.343Z
Learnt from: findolor
PR: rainlanguage/rain.strategies#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:

  • src/claims.rain
📚 Learning: 2025-07-03T09:47:14.894Z
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.

Applied to files:

  • src/claims.rain
📚 Learning: 2025-06-19T03:59:46.063Z
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.

Applied to files:

  • src/claims.rain
📚 Learning: 2025-07-15T09:19:34.859Z
Learnt from: Siddharth2207
PR: rainlanguage/rain.strategies#53
File: src/claims.rain:0-0
Timestamp: 2025-07-15T09:19:34.859Z
Learning: In Rain language, the evaluation uses a stack-based model where stack indexes start from the bottom. In the calculate-io function, stack[0] represents the ratio and stack[1] represents the amount. This means when a function returns two values like `(expected-amount, 0)`, the first value is the ratio and the second value is the amount.

Applied to files:

  • src/claims.rain
📚 Learning: 2025-07-25T04:36:42.979Z
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.

Applied to files:

  • src/claims.rain
📚 Learning: 2025-07-26T15:32:06.061Z
Learnt from: 0xgleb
PR: rainlanguage/rain.strategies#45
File: settings.yaml:48-57
Timestamp: 2025-07-26T15:32:06.061Z
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.

Applied to files:

  • src/claims.rain
🔇 Additional comments (1)
registry (1)

6-6: Pinned by full commit SHA — good.

Using a full SHA for the new claims entry matches the repo’s established pattern for registry URLs (per retrieved learnings). No action needed.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (8)
src/claims.rain (8)

17-20: Placeholder subgraph URLs.

Confirm placeholders are acceptable until deployment or add TODOs with intended endpoints.


26-36: Address casing consistency (EIP-55).

Base orderbook address is lowercase while others are checksummed; align with repo convention.


175-188: Re-assert not-claimed during fill to close TOCTOU.

Add the not-claimed check in handle-io so concurrent fills can’t double-claim between quote and fill.

Apply:

 #handle-io
   expected-amount: signed-context<0 1>(),
   leaf-node: call<'get-leaf-node>(),
+  :call<'check-claim>(leaf-node),
   expected-scaled-back: scale-n-dynamic(output-token-decimals() expected-amount),
   expected-rescale: scale-18-dynamic(output-token-decimals() expected-scaled-back),

175-186: Also assert no input tokens were provided.

Strengthen invariants; reject any accidental/malicious input.

Apply:

 #handle-io
   expected-amount: signed-context<0 1>(),
   leaf-node: call<'get-leaf-node>(),
+  :ensure(
+      equal-to(input-vault-increase() 0)
+      "No input allowed for claims"
+  ),
   expected-scaled-back: scale-n-dynamic(output-token-decimals() expected-amount),
   expected-rescale: scale-18-dynamic(output-token-decimals() expected-scaled-back),

131-139: Make the error message explicit.

Clearer for users and logs.

-    "Claimed"
+    "Already claimed"

160-163: Improve proof failure message.

More actionable error.

-    "Proof"
+    "Invalid proof"

119-130: Add domain separation to the leaf hash.

Prevents cross-context collisions and future-proofs encoding.

 #get-leaf-node
+  leaf-domain: "CLAIM_LEAF_V1",
@@
-  _: hash(signed-context<0 0>() order-counterparty() signed-context<0 1>());
+  _: hash(leaf-domain signed-context<0 0>() order-counterparty() signed-context<0 1>());

149-159: Depth-8 proof is a hard cap; verify this meets expected drop sizes.

If larger trees are possible, consider variable depth.

Would you like a patch that consumes a dynamic number of siblings from signed-context and loops verify?

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 90ce3f1 and ded7592.

📒 Files selected for processing (2)
  • registry (1 hunks)
  • src/claims.rain (1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-07-26T15:31:37.916Z
Learnt from: 0xgleb
PR: rainlanguage/rain.strategies#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-07-03T09:47:48.019Z
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.

Applied to files:

  • registry
📚 Learning: 2025-09-01T12:19:42.343Z
Learnt from: findolor
PR: rainlanguage/rain.strategies#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:

  • src/claims.rain
📚 Learning: 2025-07-03T09:47:14.894Z
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.

Applied to files:

  • src/claims.rain
📚 Learning: 2025-06-19T03:59:46.063Z
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.

Applied to files:

  • src/claims.rain
📚 Learning: 2025-07-15T09:19:34.859Z
Learnt from: Siddharth2207
PR: rainlanguage/rain.strategies#53
File: src/claims.rain:0-0
Timestamp: 2025-07-15T09:19:34.859Z
Learning: In Rain language, the evaluation uses a stack-based model where stack indexes start from the bottom. In the calculate-io function, stack[0] represents the ratio and stack[1] represents the amount. This means when a function returns two values like `(expected-amount, 0)`, the first value is the ratio and the second value is the amount.

Applied to files:

  • src/claims.rain
📚 Learning: 2025-07-25T04:36:42.979Z
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.

Applied to files:

  • src/claims.rain
📚 Learning: 2025-07-26T15:32:06.061Z
Learnt from: 0xgleb
PR: rainlanguage/rain.strategies#45
File: settings.yaml:48-57
Timestamp: 2025-07-26T15:32:06.061Z
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.

Applied to files:

  • src/claims.rain
🔇 Additional comments (2)
registry (2)

6-6: Good: strategy pinned by full commit SHA.

Matches established registry pattern.


6-6: Pinned URL resolves successfully (HTTP 200).

Comment on lines +165 to +174
#calculate-io
using-words-from raindex-subparser merkle-words

expected-amount: signed-context<0 1>(),
leaf-node: call<'get-leaf-node>(),
:call<'check-claim>(leaf-node),
:call<'verify-proof>(leaf-node),
_: expected-amount,
_: 0;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Document calculate-io tuple semantics to avoid confusion.

Clarify that return is (ratio, amount) and why the chosen values are intentional.

 #calculate-io
   using-words-from raindex-subparser merkle-words
-
+  # Returns (ratio, amount). Intentional: ratio=expected-amount, amount=0.
   expected-amount: signed-context<0 1>(),
📝 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.

Suggested change
#calculate-io
using-words-from raindex-subparser merkle-words
expected-amount: signed-context<0 1>(),
leaf-node: call<'get-leaf-node>(),
:call<'check-claim>(leaf-node),
:call<'verify-proof>(leaf-node),
_: expected-amount,
_: 0;
#calculate-io
using-words-from raindex-subparser merkle-words
# Returns (ratio, amount). Intentional: ratio=expected-amount, amount=0.
expected-amount: signed-context<0 1>(),
leaf-node: call<'get-leaf-node>(),
:call<'check-claim>(leaf-node),
:call<'verify-proof>(leaf-node),
_: expected-amount,
_: 0;

Comment on lines +168 to +170
expected-amount: signed-context<0 1>(),
leaf-node: call<'get-leaf-node>(),
:call<'check-claim>(leaf-node),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

State units for expected-amount.

Avoids mismatches between proof encoding and vault math.

-  expected-amount: signed-context<0 1>(),
+  # expected-amount is 18-decimal fixed-point to match proof encoding.
+  expected-amount: signed-context<0 1>(),
📝 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.

Suggested change
expected-amount: signed-context<0 1>(),
leaf-node: call<'get-leaf-node>(),
:call<'check-claim>(leaf-node),
# expected-amount is 18-decimal fixed-point to match proof encoding.
expected-amount: signed-context<0 1>(),
leaf-node: call<'get-leaf-node>(),
:call<'check-claim>(leaf-node),
🤖 Prompt for AI Agents
In src/claims.rain around lines 168-170, the expected-amount field should be
declared as a state unit to prevent mismatches between proof encoding and vault
math; change the field from a raw signed-context declaration to a state-unit
that uses the signed-context (0,1) typing and update any dependent references to
use the new state-unit accessor semantics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants