Repository navigation
Conversation
0xgleb
approved these changes
Apr 6, 2026
Document vault-id: vaultless for V6 orderbook wallet-based trading instead of a separate vaultless boolean. Includes field definition, validation rules, and examples for vaultless and hybrid orders.
Walkthrough
ChangesVaultless Mode Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
findolor
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ob-yaml.md`:
- Around line 332-338: The vaultless-order example has a mismatched
configuration where the input uses vault-id: vaultless but the output uses
vault-id: 1, making it a hybrid order rather than a pure vaultless order as the
name suggests. To align with the PR objectives, change the output section of the
vaultless-order example to use vault-id: vaultless instead of vault-id: 1,
ensuring all inputs and outputs in this example consistently use the vaultless
vault-id.
- Around line 282-305: Replace imperative and descriptive language with RFC 2119
keywords throughout the Input/Output fields, Vaultless mode, and Validation
rules sections to meet specification document standards. In the Input/Output
fields section, change "Required fields:" to "The following fields are
REQUIRED:" and "Optional fields:" to "The following fields are OPTIONAL:". In
the Vaultless mode section, convert statements like "the order uses the owner's
wallet directly" to "the order MUST use the owner's wallet directly" and "Tokens
received are sent directly" to "Tokens received MUST be sent directly". Apply
the same RFC 2119 transformations (MUST, SHOULD, MAY) throughout all behavioral
rules and validation scenarios to ensure unambiguous specification precision.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+282
to
+305
| #### Input/Output fields | ||
|
|
||
| Required fields: | ||
| - `token` (foreign key into the tokens mapping) | ||
|
|
||
| Optional fields: | ||
| - `vault-id` (vault identifier, generates random if omitted; set to `vaultless` to use the owner wallet directly instead of a vault) | ||
|
|
||
| #### Vaultless mode | ||
|
|
||
| When `vault-id: vaultless`, the order uses the owner's wallet directly instead of a vault: | ||
| - **Vaultless inputs**: Tokens received are sent directly to the owner's wallet | ||
| - **Vaultless outputs**: Tokens given are pulled directly from the owner's wallet (requires approval) | ||
| - **Hybrid orders**: Each input/output can independently be vaultless or vault-based | ||
|
|
||
| ##### Validation rules | ||
|
|
||
| | Scenario | Result | | ||
| |----------|--------| | ||
| | `vault-id: vaultless` | Valid - uses wallet directly | | ||
| | `vault-id: 0` | Error: "Invalid vault-id value. For vaultless mode use vault-id: vaultless" | | ||
| | `vault-id` omitted | Generates random vault ID | | ||
| | numeric or hex `vault-id` | Uses the specified vault ID | | ||
|
|
There was a problem hiding this comment.
Use RFC 2119 language for specification precision.
The specification section (Input/Output fields and Vaultless mode) uses imperative and descriptive language, but as a specification document, it should use RFC 2119 keywords (MUST, SHOULD, MAY) for unambiguous precision. This is required by your coding guidelines for specification documents.
For example:
- "Required fields:" should be "The following fields are REQUIRED:"
- Behavioral rules like "When
vault-id: vaultless, the order uses..." should state "Whenvault-idis set tovaultless, the order MUST use..." - "Tokens received are sent directly..." should be "Tokens received MUST be sent directly..."
[mandatory_style]
📋 Proposed fix: Apply RFC 2119 language
#### Input/Output fields
-Required fields:
-The `token` field is REQUIRED (foreign key into the tokens mapping)
+**The following field is REQUIRED:**
+- `token` (foreign key into the tokens mapping)
-Optional fields:
-The `vault-id` field is OPTIONAL (vault identifier, generates random if omitted; set to `vaultless` to use the owner wallet directly instead of a vault)
+**The following field is OPTIONAL:**
+- `vault-id` (vault identifier; MUST generate a random ID if omitted; set to `vaultless` to use the owner wallet directly instead of a vault)
#### Vaultless mode
-When `vault-id: vaultless`, the order uses the owner's wallet directly instead of a vault:
-- **Vaultless inputs**: Tokens received are sent directly to the owner's wallet
-- **Vaultless outputs**: Tokens given are pulled directly from the owner's wallet (requires approval)
-- **Hybrid orders**: Each input/output can independently be vaultless or vault-based
+When `vault-id` is set to `vaultless`, the order MUST use the owner's wallet directly instead of a vault:
+- **Vaultless inputs**: Tokens received MUST be sent directly to the owner's wallet
+- **Vaultless outputs**: Tokens given MUST be pulled directly from the owner's wallet (requires approval for the wallet owner)
+- **Hybrid orders**: Each input/output MAY independently use vaultless or vault-based mode🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ob-yaml.md` around lines 282 - 305, Replace imperative and descriptive
language with RFC 2119 keywords throughout the Input/Output fields, Vaultless
mode, and Validation rules sections to meet specification document standards. In
the Input/Output fields section, change "Required fields:" to "The following
fields are REQUIRED:" and "Optional fields:" to "The following fields are
OPTIONAL:". In the Vaultless mode section, convert statements like "the order
uses the owner's wallet directly" to "the order MUST use the owner's wallet
directly" and "Tokens received are sent directly" to "Tokens received MUST be
sent directly". Apply the same RFC 2119 transformations (MUST, SHOULD, MAY)
throughout all behavioral rules and validation scenarios to ensure unambiguous
specification precision.
Source: Coding guidelines
Comment on lines
+332
to
+338
| vaultless-order: | ||
| inputs: | ||
| - token: eth-weth | ||
| vault-id: vaultless | ||
| outputs: | ||
| - token: eth-usdc | ||
| vault-id: 1 |
There was a problem hiding this comment.
Mismatch between example name and content.
The example named vaultless-order has a vaultless input but a vault-based output (vault-id: 1), making it a hybrid order, not a pure vaultless order. This contradicts the PR objectives which state that vaultless-order should have "all inputs/outputs use vault-id: vaultless".
Consider either:
- Rename
vaultless-orderto better reflect its hybrid nature, or - Create a true vaultless example where all inputs/outputs use
vault-id: vaultless
The existing hybrid-order (lines 339–347) correctly demonstrates mixing vaultless and vault-based modes.
🔧 Proposed fix: Create a true vaultless example
vaultless-order:
inputs:
- token: eth-weth
vault-id: vaultless
outputs:
- token: eth-usdc
- vault-id: 1
+ vault-id: vaultless
hybrid-order:
inputs:
- token: eth-weth
vault-id: vaultless
outputs:
- token: eth-usdc
vault-id: vaultless
- token: eth-dai
vault-id: 0x123🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ob-yaml.md` around lines 332 - 338, The vaultless-order example has a
mismatched configuration where the input uses vault-id: vaultless but the output
uses vault-id: 1, making it a hybrid order rather than a pure vaultless order as
the name suggests. To align with the PR objectives, change the output section of
the vaultless-order example to use vault-id: vaultless instead of vault-id: 1,
ensuring all inputs and outputs in this example consistently use the vaultless
vault-id.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Document the new
vault-id: vaultlesssentinel for order inputs/outputs in the YAML spec. This enables direct wallet-based trading in V6 orderbook without adding a second vault-related field.Parent issue: rainlanguage/raindex#2402
Solution
Added documentation to
ob-yaml.mdfor usingvaultlessas a sentinel value ofvault-id:token) and optional (vault-id) fieldsvault-id: vaultless, omittedvault-id, numeric/hex vault IDs, and invalidvault-id: 0vaultless-orderandhybrid-orderexamples usingvault-id: vaultlessChecks
By submitting this for review, I'm confirming I've done the following:
fixes #43
Summary by CodeRabbit
vault-idvalues.