Fix deposit null pointer error

[rouzwelt]

Motivation

resolves #2129

This PR fixes the null pointer error when trying to deposit into a vault, this happens because Float wasm class ownership is given to the rust function when creating approve tx and ten same instance which is now a null pointer is given to deposit tx creation method.

Solution

To fix this, we should simply use refs for Float args in deposit, approval and similar method that use it to avoid ownership transfer and as a result the instance getting dropped once the owner's scope ends.

from vaults page:

Test.Deposit.Withdraw.mov

from order page:

Order.Page.Deposit.Withdraw.Test.mov

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

• Refactor

  • Internal vault operations updated to use borrowed amount inputs; behavior and public outputs unchanged.
  • Multi-vault withdrawal flow aligned with the same internal calling convention; end-user results remain consistent.

• Tests

  • Updated tests to reflect the new parameter passing semantics across deposit/withdraw/approval paths.

[coderabbitai]

Walkthrough

Function signatures in RaindexVault were changed to accept borrowed &Float references; internal code and call sites (including VaultsList) were updated to use dereferenced values where ownership is needed. Return types and external behavior remain unchanged.

Changes

Cohort / File(s)	Summary
RaindexVault API signature updates crates/common/src/raindex_client/vaults.rs	Change method parameters from Float to &Float for validate_amount, get_deposit_calldata, get_withdraw_calldata, get_approval_calldata, and get_deposit_and_transaction_args; adjust internals to dereference/cloned values where ownership is required; update allowance comparison to use *amount; update zero-amount path to pass a borrowed Float.
VaultsList call site update crates/common/src/raindex_client/vaults_list.rs	Update call to vault.get_withdraw_calldata(&vault.balance()).await to pass a reference; surrounding control flow and multicall encoding unchanged.
Tests / call-site updates ...tests...	Tests updated to pass &Float references to deposit/withdraw/approval calldata calls.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Assessment against linked issues

Objective	Addressed	Explanation
Fix null pointer error when depositing/withdrawing to/from vaults on web (#2129)	❓	Signature changes remove owned-argument crossing WASM boundary, but repository-level JS/WASM bindings and caller changes are not shown, so impact on the null pointer error is unclear.

Possibly related PRs

• Fix allowance check for deposits #1901 — Modifies approval/allowance comparison and adjusts signatures to use &Float, closely related to allowance logic changes here.
• Add vault related data structures and functions  #1950 — Edits the same RaindexVault methods and call sites; likely touches related codepaths.

Suggested labels

bug

Suggested reviewers

• findolor
• hardyjosh

---

📜 Recent 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 ef8fa7a and df31ac0.

📒 Files selected for processing (1)

• crates/common/src/raindex_client/vaults.rs (14 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

crates/**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Keep Rust libraries and tooling inside crates/

Files:

• crates/common/src/raindex_client/vaults.rs

🧠 Learnings (7)

📓 Common learnings

Learnt from: findolor
PR: rainlanguage/rain.orderbook#2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1941
File: crates/js_api/src/raindex/vaults.rs:175-181
Timestamp: 2025-06-20T07:51:08.790Z
Learning: In the RaindexClient vault methods, `&self` parameters are intentionally kept for API consistency to make all vault operations instance methods, even when the methods don't use client state. This is a design preference for maintaining a uniform interface.

Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1911
File: crates/subgraph/src/types/impls.rs:7-15
Timestamp: 2025-07-21T16:34:31.193Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb considers breaking changes that remove unsafe default behaviors to be intentional and acceptable. Specifically, the get_decimals() method in crates/subgraph/src/types/impls.rs was intentionally changed to return MissingDecimals error instead of defaulting to 18 decimals, as defaulting to 18 is considered unsafe and should never have been done.

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1994
File: crates/common/src/raindex_client/vaults.rs:59-59
Timestamp: 2025-07-16T05:52:05.576Z
Learning: User findolor prefers to handle documentation updates for getter methods in batch via dedicated PRs rather than addressing them individually during feature development, as mentioned for the formatted amount string fields in crates/common/src/raindex_client/vaults.rs.

Learnt from: brusherru
PR: rainlanguage/rain.orderbook#2044
File: packages/orderbook/README.md:186-189
Timestamp: 2025-08-01T09:07:20.383Z
Learning: In the rainlanguage/rain.orderbook project, Rust methods on structs like RaindexVaultsList are exported as JavaScript getters in WASM bindings using #[wasm_bindgen(getter)]. This means while the Rust code uses method calls like items(), the JavaScript/WASM API exposes them as property access like .items. The README.md correctly documents the JavaScript API surface, not the Rust implementation details.

📚 Learning: 2025-07-16T10:40:05.717Z

Learnt from: findolor
PR: rainlanguage/rain.orderbook#2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.

Applied to files:

• crates/common/src/raindex_client/vaults.rs

📚 Learning: 2025-07-16T05:52:05.576Z

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1994
File: crates/common/src/raindex_client/vaults.rs:59-59
Timestamp: 2025-07-16T05:52:05.576Z
Learning: User findolor prefers to handle documentation updates for getter methods in batch via dedicated PRs rather than addressing them individually during feature development, as mentioned for the formatted amount string fields in crates/common/src/raindex_client/vaults.rs.

Applied to files:

• crates/common/src/raindex_client/vaults.rs

📚 Learning: 2025-06-20T07:51:08.790Z

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1941
File: crates/js_api/src/raindex/vaults.rs:175-181
Timestamp: 2025-06-20T07:51:08.790Z
Learning: In the RaindexClient vault methods, `&self` parameters are intentionally kept for API consistency to make all vault operations instance methods, even when the methods don't use client state. This is a design preference for maintaining a uniform interface.

Applied to files:

• crates/common/src/raindex_client/vaults.rs

📚 Learning: 2025-07-21T16:34:31.193Z

Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1911
File: crates/subgraph/src/types/impls.rs:7-15
Timestamp: 2025-07-21T16:34:31.193Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb considers breaking changes that remove unsafe default behaviors to be intentional and acceptable. Specifically, the get_decimals() method in crates/subgraph/src/types/impls.rs was intentionally changed to return MissingDecimals error instead of defaulting to 18 decimals, as defaulting to 18 is considered unsafe and should never have been done.

Applied to files:

• crates/common/src/raindex_client/vaults.rs

📚 Learning: 2025-08-01T09:07:20.383Z

Learnt from: brusherru
PR: rainlanguage/rain.orderbook#2044
File: packages/orderbook/README.md:186-189
Timestamp: 2025-08-01T09:07:20.383Z
Learning: In the rainlanguage/rain.orderbook project, Rust methods on structs like RaindexVaultsList are exported as JavaScript getters in WASM bindings using #[wasm_bindgen(getter)]. This means while the Rust code uses method calls like items(), the JavaScript/WASM API exposes them as property access like .items. The README.md correctly documents the JavaScript API surface, not the Rust implementation details.

Applied to files:

• crates/common/src/raindex_client/vaults.rs

📚 Learning: 2025-05-09T05:30:02.520Z

Learnt from: findolor
PR: rainlanguage/rain.orderbook#1744
File: packages/webapp/src/lib/components/DepositOrWithdrawModal.svelte:73-76
Timestamp: 2025-05-09T05:30:02.520Z
Learning: In Rain.orderbook, `VaultCalldataResult` is a wrapper type that contains both `error` and `value` properties. When using this in the `DepositOrWithdrawModal` component, only the inner `value` field is passed to the `handleTransaction` function, not the complete wrapper.

Applied to files:

• crates/common/src/raindex_client/vaults.rs

🧬 Code graph analysis (1)

crates/common/src/raindex_client/vaults.rs (1)

crates/common/src/withdraw.rs (1)

• from (21-28)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)

• GitHub Check: build-tauri (ubuntu-22.04, true)
• GitHub Check: git-clean
• GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
• GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
• GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
• GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
• GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
• GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
• GitHub Check: test
• GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
• GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
• GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
• GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
• GitHub Check: test
• GitHub Check: test
• GitHub Check: Deploy-Preview-Push
• GitHub Check: Deploy-Docs-Preview

🔇 Additional comments (6)

crates/common/src/raindex_client/vaults.rs (6)

392-406: Withdraw now borrows Float; correct and prevents ownership move.

Setting target_amount: *amount is appropriate given Float: Copy.

---

408-434: Internal arg builder now borrows; good cohesion.

No functional regressions; keeps &self per existing API design preference.

---

1948-1970: Tests: deposit calldata updated for borrowed Float — good.

Covers positive amount and zero-amount error path.

---

2013-2033: Tests: withdraw calldata updated — good.

Validates tiny amounts and zero-amount guard.

---

2076-2107: Tests: approval calldata updated — good coverage.

Checks zero, sufficient allowance, and insufficient allowance cases.

---

329-369: No remaining owned Float parameters in WASM‐exported or RaindexVault methods
Scanned the entire crates directory (including all #[wasm_export] functions) and all RaindexVault impls for any methods taking Float by value—none were found.

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch 2025-09-06-fix-deposit-null-pointer-err

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment:

S/M/L PR Classification Guidelines:

This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed.

Small (S)

Characteristics:

• Simple bug fixes, typos, or minor refactoring
• Single-purpose changes affecting 1-2 files
• Documentation updates
• Configuration tweaks
• Changes that require minimal context to review

Review Effort: Would have taken 5-10 minutes

Examples:

• Fix typo in variable name
• Update README with new instructions
• Adjust configuration values
• Simple one-line bug fixes
• Import statement cleanup

Medium (M)

Characteristics:

• Feature additions or enhancements
• Refactoring that touches multiple files but maintains existing behavior
• Breaking changes with backward compatibility
• Changes requiring some domain knowledge to review

Review Effort: Would have taken 15-30 minutes

Examples:

• Add new feature or component
• Refactor common utility functions
• Update dependencies with minor breaking changes
• Add new component with tests
• Performance optimizations
• More complex bug fixes

Large (L)

Characteristics:

• Major feature implementations
• Breaking changes or API redesigns
• Complex refactoring across multiple modules
• New architectural patterns or significant design changes
• Changes requiring deep context and multiple review rounds

Review Effort: Would have taken 45+ minutes

Examples:

• Complete new feature with frontend/backend changes
• Protocol upgrades or breaking changes
• Major architectural refactoring
• Framework or technology upgrades

Additional Factors to Consider

When deciding between sizes, also consider:

• Test coverage impact: More comprehensive test changes lean toward larger classification
• Risk level: Changes to critical systems bump up a size category
• Team familiarity: Novel patterns or technologies increase complexity

Notes:

• the assessment must be for the totality of the PR, that means comparing the base branch to the last commit of the PR
• the assessment output must be exactly one of: S, M or L (single-line comment) in format of: SIZE={S/M/L}
• do not include any additional text, only the size classification
• your assessment comment must not include tips or additional sections
• do NOT tag me or anyone else on your comment

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

I need to gather more detailed information about the PR changes to make an accurate size assessment.

SIZE=M

---

🏁 Script executed:

#!/bin/bash
# Get the total diff stats for the PR, excluding lock files
git diff --stat HEAD~$(git rev-list --count HEAD ^origin/main) | grep -v -E '\.(lock|json)$|package-lock|yarn\.lock|pnpm-lock|cargo\.lock' | tail -1
# Get detailed file changes
git diff --stat HEAD~$(git rev-list --count HEAD ^origin/main) --
# Count total lines changed (additions + deletions)
git diff --numstat HEAD~$(git rev-list --count HEAD ^origin/main) | grep -v -E '\.(lock|json)$|package-lock|yarn\.lock|pnpm-lock|cargo\.lock' | awk '{additions += $1; deletions += $2} END {print "TOTAL=" (additions + deletions) "\nADDITIONS=" additions "\nDELETIONS=" deletions}'

Length of output: 945

---

🏁 Script executed:

#!/bin/bash
# Check current branch and available branches
echo "Current branch:"
git branch --show-current
echo ""
echo "Available branches:"
git branch -a
echo ""
# Try direct comparison with origin/main
echo "Diff stats with origin/main:"
git diff --stat origin/main 2>/dev/null || echo "origin/main not available"
echo ""
# Get commit info
echo "Recent commits:"
git log --oneline -5
echo ""
# Try comparing with main if it exists
git diff --stat main 2>/dev/null || echo "main branch not available"

Length of output: 534

---

🏁 Script executed:

#!/bin/bash
# Get complete diff stats
echo "=== Diff Statistics ==="
git diff --stat origin/main

echo ""
echo "=== Numerical Stats ==="
git diff --numstat origin/main

echo ""
echo "=== Files Changed ==="
git diff --name-only origin/main

echo ""
echo "=== Summary of changes ==="
git diff --shortstat origin/main

Length of output: 395

---

🏁 Script executed:

#!/bin/bash
# Get more detailed commit history
echo "=== Detailed Commit History ==="
git log --oneline --graph -10

echo ""
echo "=== Check if there are any uncommitted changes ==="
git status

echo ""
echo "=== Try to find the PR commits ==="
git log --grep="Fix deposit null pointer" --oneline

echo ""
echo "=== Show commits that are in current branch but not in origin/main ==="
git log origin/main..HEAD --oneline

echo ""
echo "=== Compare branches directly ==="
git diff origin/main...HEAD --name-only

Length of output: 966

---

🏁 Script executed:

#!/bin/bash
# Look at the merge commit in detail
echo "=== Merge Commit Details ==="
git show --stat a984f85

echo ""
echo "=== Look for recent commits by time ==="
git log --since="2025-09-05" --until="2025-09-07" --oneline --all

echo ""
echo "=== Check all branches for the source branch ==="
git branch -a | grep -i deposit

echo ""
echo "=== Look at the merge commit parents ==="
git show --format=fuller a984f85

echo ""
echo "=== Check if there are any other branches with changes ==="
git log --all --oneline --grep="deposit" -i

Length of output: 265951

---

Based on the PR analysis, I can see this is a bug fix that systematically changes method signatures from owned Float parameters to borrowed &Float references across vault-related functionality. The changes affect method signatures in RaindexVault and update internal usage accordingly.

While this is a bug fix, it involves:

• Multiple method signature changes (public API modifications)
• Systematic updates across related code
• Rust ownership/borrowing domain knowledge required
• Changes affecting multiple files with coordinated updates
• Would require 15-30 minutes to review properly

This goes beyond a simple one-line fix but isn't a major architectural overhaul.

SIZE=M

---

🧠 Learnings used

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:42.275Z
Learning: For PR #1884, after each commit review, I should provide a new comment with a PR size report in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding irrelevant files such as lock files (package-lock.json, cargo.lock, etc.).

---

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:42.275Z
Learning: After each commit in PR #1884, I should provide a new comment with the PR size report in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`. The report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).

---

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).

---

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the exact format: "TOTAL=number\nADDITIONS=number\nDELETIONS=number". This report should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).

---

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.

---

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.

---

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:46:08.530Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report should be the only content in the comment - no text before it, no text after it, no formatting blocks, just the raw report in the exact format: "TOTAL=number\nADDITIONS=number\nDELETIONS=number". The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).

---

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.

---

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.

---

Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.