Skip to content

fix: deposit() updates internal balance but never transfers tokens on-chain #55

Description

@Uchechukwu-Ekezie

Problem

The deposit function in contracts/src/portfolio.rs updates the portfolio's internal balance record but doesn't actually transfer the tokens from the caller to the contract. After calling deposit, the portfolio shows a balance that doesn't match the contract's real token holdings.

Current flow:

  1. User calls deposit(portfolio_id, token_address, amount)
  2. Contract updates asset.balance += amount in storage
  3. Done — no token transfer happens

This means:

  • The portfolio thinks it has funds it doesn't actually hold
  • When execute_rebalance tries to swap tokens via the DEX, it'll fail because the contract doesn't have the tokens
  • A user could deposit, get credit, withdraw to a different address, and the portfolio still shows the old balance

Proposed Fix

The deposit function needs to call the Soroban token contract's transfer function to move tokens from the caller to the portfolio contract.

fn deposit(env: Env, portfolio_id: BytesN<32>, token_address: Address, amount: i128) {
    let caller = env.auths().first().unwrap();
    let mut portfolio = Self::get_portfolio(&env, &portfolio_id);
    
    // Verify the caller owns this portfolio
    assert_eq!(portfolio.owner, caller, "not portfolio owner");
    assert!(amount > 0, "amount must be positive");
    
    // Transfer tokens from caller to this contract
    let token_client = token::Client::new(&env, &token_address);
    token_client.transfer(&caller, &env.current_contract_address(), &amount);
    
    // Now update internal balance
    let asset = portfolio.assets.iter_mut()
        .find(|a| a.token_address == token_address)
        .expect("token not in portfolio");
    asset.balance += amount;
    
    // Recalculate total_value (depends on #49 being fixed)
    portfolio.total_value = Self::calculate_total_value(&env, &portfolio);
    
    Self::save_portfolio(&env, &portfolio);
}

Key details

  • Use token::Client from the Soroban token interface to call transfer. The token contract address is already stored in each asset.
  • The transfer must happen before the balance update — if the transfer fails (insufficient balance, not authorized), the whole transaction should revert.
  • The caller needs to have authorized the contract to spend their tokens. For Soroban token contracts, this means the caller must have called approve on the token contract for this portfolio's contract address, OR the transfer must be initiated by the caller (which it is, since they're calling deposit).
  • Handle the case where the token contract doesn't exist or isn't a valid Soroban token — token_client.transfer will panic, which is fine since it'll revert the whole transaction.

Testing

  • Deposit XLM to a portfolio → verify the contract's XLM balance increases
  • Deposit USDC to a portfolio → verify the contract's USDC balance increases
  • Deposit with insufficient balance → should revert, portfolio unchanged
  • Deposit a token not in the portfolio → should revert with "token not in portfolio"
  • Deposit zero amount → should revert

Files to modify

  • contracts/src/portfolio.rs — add token transfer call in deposit
  • contracts/src/lib.rs — import the token client
  • contracts/src/test.rs — add tests for actual token transfers
  • contracts/Cargo.toml — ensure soroban-sdk token features are enabled

Acceptance Criteria

  • deposit transfers tokens from caller to contract via Soroban token interface
  • Internal balance matches actual on-chain token holdings after deposit
  • Insufficient balance reverts the entire transaction
  • Portfolio owner verification still works
  • Multi-asset deposits work (deposit XLM, then USDC, then XLM again)
  • Unit tests verify actual token balance changes, not just internal state

References

Affected Area

Smart Contracts

Checklist

  • I have searched existing issues and this is not a duplicate

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcontractSmart contract relatedhelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions