Add vaultless support to GUI order operations #2413
Conversation
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
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR implements vaultless order operations support in the GUI layer. Changes include: adding Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI/Client
participant GUI as DotrainOrderGui
participant ERC20 as ERC20 Contract
participant OB as Orderbook
UI->>GUI: get_deployment_transaction_args(owner, vaultless_approval_amounts)
GUI->>GUI: Parse deployment & discover vaultless outputs
alt Vaultless outputs found
loop For each vaultless token
GUI->>GUI: Resolve approval amount (custom or max uint256)
GUI->>ERC20: allowance(owner, orderbook)
ERC20-->>GUI: current_allowance
alt current_allowance != target_amount
GUI->>GUI: Generate approval calldata
else
GUI->>GUI: Skip approval (already set)
end
end
end
GUI->>GUI: Skip deposit generation for vaultless inputs
GUI-->>UI: Return deployment args with approval calldatas
UI->>OB: Execute approval transactions
UI->>OB: Execute order deployment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
1 similar comment
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/js_api/src/gui/order_operations.rs (1)
343-435: Only skip deposit approval for tokens that are exclusively vaultless, not when they appear in both vaulted and vaultless outputs.A token used in both a vaulted output and a vaultless output requires deposit approval for the vaulted portion. The current check skips approval whenever a token appears in any vaultless output, causing insufficient allowance for vaulted deposits. Track vaulted addresses separately and only skip when a token is exclusively vaultless.
🔧 Suggested fix (track vaulted vs vaultless addresses)
-use std::{collections::HashMap, str::FromStr, sync::Arc}; +use std::{collections::{HashMap, HashSet}, str::FromStr, sync::Arc}; @@ - let vaultless_addresses: Vec<Address> = vaultless_outputs + let vaultless_addresses: Vec<Address> = vaultless_outputs .iter() .filter_map(|output| output.token.as_ref().map(|t| t.address)) .collect(); + let vaulted_addresses: HashSet<Address> = deployment + .deployment + .order + .outputs + .iter() + .filter(|output| output.vaultless != Some(true)) + .filter_map(|output| output.token.as_ref().map(|t| t.address)) + .collect(); @@ - if vaultless_addresses.contains(token_address) { + if vaultless_addresses.contains(token_address) + && !vaulted_addresses.contains(token_address) + { continue; }
🤖 Fix all issues with AI agents
In `@packages/orderbook/README.md`:
- Around line 471-473: Add a commented example showing how to pass the optional
Map parameter to getDeploymentTransactionArgs (e.g., a Map of token keys to
custom approval amounts) so readers can see vaultless approval usage; create a
variable whose name matches the actual parameter name in the API (use the
verified parameter name for getDeploymentTransactionArgs) and show calling
getDeploymentTransactionArgs('0xOwner', <thatVariable>) with a brief inline
comment explaining that omitting or passing undefined yields infinite approval
(max uint256).
- Around line 463-466: Update the README example to use the correct parameter
name expected by the API: replace the local variable vaultlessApprovals with
vaultlessApprovalAmounts and pass vaultlessApprovalAmounts into
gui.generateApprovalCalldatas (referencing the example variables and the
generateApprovalCalldatas function) so the doc matches the implementation.
packages/orderbook/README.md
Outdated
Comment on lines
463
to
466
| // For vaultless outputs, you can optionally specify custom approval amounts per token. | ||
| // By default, vaultless outputs use infinite approval (max uint256). | ||
| // const vaultlessApprovals = new Map([['output-token', '1000000000000000000']]); | ||
| // const approvalCalldatasResult = await gui.generateApprovalCalldatas('0xOwner', vaultlessApprovals); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the vaultless approval parameter in the implementation
rg -n "vaultless.*approval" --type rust crates/js_api/src/gui/order_operations.rs -C 3Repository: rainlanguage/rain.orderbook
Length of output: 2808
Update parameter name in documentation example.
The example uses vaultlessApprovals, but the actual API parameter is vaultlessApprovalAmounts. Update the variable name to match the implementation:
Show diff
// For vaultless outputs, you can optionally specify custom approval amounts per token.
// By default, vaultless outputs use infinite approval (max uint256).
-// const vaultlessApprovals = new Map([['output-token', '1000000000000000000']]);
-// const approvalCalldatasResult = await gui.generateApprovalCalldatas('0xOwner', vaultlessApprovals);
+// const vaultlessApprovalAmounts = new Map([['output-token', '1000000000000000000']]);
+// const approvalCalldatasResult = await gui.generateApprovalCalldatas('0xOwner', vaultlessApprovalAmounts);
🤖 Prompt for AI Agents
In `@packages/orderbook/README.md` around lines 463 - 466, Update the README
example to use the correct parameter name expected by the API: replace the local
variable vaultlessApprovals with vaultlessApprovalAmounts and pass
vaultlessApprovalAmounts into gui.generateApprovalCalldatas (referencing the
example variables and the generateApprovalCalldatas function) so the doc matches
the implementation.
Comment on lines
471
to
475
| // For vaultless outputs, pass an optional Map of token keys to custom approval amounts. | ||
| // Omit or pass undefined for infinite approval (max uint256) on vaultless tokens. | ||
| const deploymentArgsResult = await gui.getDeploymentTransactionArgs('0xOwner'); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add example showing the optional parameter usage.
While the comment mentions passing an optional Map to getDeploymentTransactionArgs, no example demonstrates this. Consider adding a commented example similar to lines 463-466:
// For vaultless outputs, pass an optional Map of token keys to custom approval amounts.
// Omit or pass undefined for infinite approval (max uint256) on vaultless tokens.
+// const vaultlessApprovals = new Map([['output-token', '1000000000000000000']]);
+// const deploymentArgsResult = await gui.getDeploymentTransactionArgs('0xOwner', vaultlessApprovals);
const deploymentArgsResult = await gui.getDeploymentTransactionArgs('0xOwner');Note: Ensure the variable name matches the verified parameter name from the API.
📝 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
| // For vaultless outputs, pass an optional Map of token keys to custom approval amounts. | |
| // Omit or pass undefined for infinite approval (max uint256) on vaultless tokens. | |
| const deploymentArgsResult = await gui.getDeploymentTransactionArgs('0xOwner'); | |
| // For vaultless outputs, pass an optional Map of token keys to custom approval amounts. | |
| // Omit or pass undefined for infinite approval (max uint256) on vaultless tokens. | |
| // const vaultlessApprovals = new Map([['output-token', '1000000000000000000']]); | |
| // const deploymentArgsResult = await gui.getDeploymentTransactionArgs('0xOwner', vaultlessApprovals); | |
| const deploymentArgsResult = await gui.getDeploymentTransactionArgs('0xOwner'); |
🤖 Prompt for AI Agents
In `@packages/orderbook/README.md` around lines 471 - 473, Add a commented example
showing how to pass the optional Map parameter to getDeploymentTransactionArgs
(e.g., a Map of token keys to custom approval amounts) so readers can see
vaultless approval usage; create a variable whose name matches the actual
parameter name in the API (use the verified parameter name for
getDeploymentTransactionArgs) and show calling
getDeploymentTransactionArgs('0xOwner', <thatVariable>) with a brief inline
comment explaining that omitting or passing undefined yields infinite approval
(max uint256).
4 tasks
findolor
force-pushed
the
2025-01-20-vaultless-phase-2-sdk-validation
branch
from
08b0b59 to
995b510
Compare
findolor
force-pushed
the
2026-01-20-vaultless-phase-3-gui-order-operations
branch
from
e3e1098 to
309a484
Compare
findolor
force-pushed
the
2025-01-20-vaultless-phase-2-sdk-validation
branch
from
995b510 to
e742b2f
Compare
findolor
force-pushed
the
2026-01-20-vaultless-phase-3-gui-order-operations
branch
from
309a484 to
9b8a03d
Compare
- generateApprovalCalldatas() generates approvals for vaultless outputs - accepts optional vaultlessApprovalAmounts param for custom amounts - defaults to U256::MAX (infinite approval) for vaultless tokens - generateDepositCalldatas() skips vaultless outputs - add InvalidU256 error variant for invalid approval amounts - add vaultless test fixtures to mod.rs
Tests now generate state dynamically instead of comparing against a hardcoded SERIALIZED_STATE constant, making them more robust to changes in the serialization format.
- test infinite approval for vaultless outputs - test custom approval amounts via vaultlessApprovalAmounts param - test skipping approval when already at max - test skipping deposits for vaultless outputs - test getVaultIds returns undefined for vaultless tokens
- add vaultless mode to "How It Works" section - document optional vaultlessApprovalAmounts parameter
- use < instead of != for allowance check to avoid unnecessary approvals - fix vaultlessApprovals -> vaultlessApprovalAmounts in README example - add example for getDeploymentTransactionArgs optional parameter
Compare U256 values directly instead of converting allowance to Float. This fixes LossyConversionToFloat error when allowance is U256::MAX or any value too large for Float's 224-bit coefficient. Also changes logic to only approve when allowance < deposit amount (not when unequal).
findolor
force-pushed
the
2026-01-20-vaultless-phase-3-gui-order-operations
branch
from
9b8a03d to
99d1d11
Compare
- Rename and fix test that incorrectly expected approval when allowance > deposit - Add test for allowance = 0 case (should generate approval) - Add test for allowance = deposit case (should skip approval)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Chained PRs
Motivation
See issues:
Phase 3 of the vaultless orders implementation. When orders have
vaultless: trueon outputs, funds flow directly from the user's wallet instead of orderbook vaults. This requires the GUI to handle approvals differently (approve orderbook to spend wallet tokens) and skip deposit generation for vaultless outputs.Solution
GUI Order Operations (
order_operations.rs):generateApprovalCalldatas()now generates approvals for vaultless outputs with U256::MAX (infinite approval) by defaultvaultlessApprovalAmountsparameter (Map<tokenKey, amount>) for custom approval amounts per tokengenerateDepositCalldatas()skips vaultless outputs since they don't use vaultsgetDeploymentTransactionArgs()accepts the same optional vaultless approval amounts parameterError Handling (
mod.rs):InvalidU256error variant for invalid approval amount stringsvaultless-deployment,mixed-vault-deployment)Tests:
Documentation:
vaultlessApprovalAmountsparameterChecks
By submitting this for review, I'm confirming I've done the following:
fix #2405
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.