Skip to content

Add support for deposit of non PNL collaterals#46

Open
ex10ded wants to merge 14 commits into
starkware-libs:devfrom
x10xchange:vault_changes_1
Open

Add support for deposit of non PNL collaterals#46
ex10ded wants to merge 14 commits into
starkware-libs:devfrom
x10xchange:vault_changes_1

Conversation

@ex10ded

@ex10ded ex10ded commented Sep 15, 2025

Copy link
Copy Markdown
Collaborator

This change is Reviewable

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Deposit Cancellation Fails for Non-Base Assets

The cancel_deposit function cannot cancel deposits for non-base collateral assets. It lacks an asset_id parameter and incorrectly computes the deposit hash using the base collateral's token contract and quantum. This hash mismatch prevents cancellation of deposits made with other asset types, such as vault shares.

workspace/apps/perpetuals/contracts/src/core/components/deposit/deposit.cairo#L130-L198

/// place yet.
///
/// Validations:
/// - The deposit requested to cancel exists, is not canceled and is not processed.
/// - The cancellation delay has passed.
///
/// Execution:
/// - Transfers the quantized amount back to the user.
/// - Updates the deposit status to canceled.
/// - Emits a DepositCanceled event.
fn cancel_deposit(
ref self: ComponentState<TContractState>,
position_id: PositionId,
quantized_amount: u64,
salt: felt252,
) {
let caller_address = get_caller_address();
let assets = get_dep_component!(@self, Assets);
let token_contract = assets.get_collateral_token_contract();
let deposit_hash = deposit_hash(
token_address: token_contract.contract_address,
depositor: caller_address,
:position_id,
:quantized_amount,
:salt,
);
// Validations
match self.get_deposit_status(:deposit_hash) {
DepositStatus::PENDING(deposit_timestamp) => assert(
Time::now() > deposit_timestamp.add(self.cancel_delay.read()),
errors::DEPOSIT_NOT_CANCELABLE,
),
DepositStatus::NOT_REGISTERED => panic_with_felt252(errors::DEPOSIT_NOT_REGISTERED),
DepositStatus::PROCESSED => panic_with_felt252(errors::DEPOSIT_ALREADY_PROCESSED),
DepositStatus::CANCELED => panic_with_felt252(errors::DEPOSIT_ALREADY_CANCELED),
}
self.registered_deposits.write(key: deposit_hash, value: DepositStatus::CANCELED);
let quantum = assets.get_collateral_quantum();
let unquantized_amount = quantized_amount * quantum.into();
token_contract.transfer(recipient: caller_address, amount: unquantized_amount.into());
self
.emit(
events::DepositCanceled {
position_id,
depositing_address: caller_address,
collateral_id: assets.get_collateral_id(),
quantized_amount,
unquantized_amount,
deposit_request_hash: deposit_hash,
salt,
},
);
}
/// Process deposit a collateral amount from the 'depositing_address' to a given position.
///
/// Validations:
/// - Only the operator can call this function.
/// - The contract must not be paused.
/// - The `operator_nonce` must be valid.
/// - The `expiration` time has not passed.
/// - The collateral asset exists in the system.
/// - The collateral asset is active.
/// - The funding validation interval has not passed since the last funding tick.
/// - The prices of all assets in the system are valid.
/// - The deposit message has not been fulfilled.

workspace/apps/perpetuals/contracts/src/core/components/deposit/interface.cairo#L18-L22

);
fn cancel_deposit(
ref self: TContractState, position_id: PositionId, quantized_amount: u64, salt: felt252,
);
fn process_deposit(

Fix in Cursor Fix in Web


@ex10ded ex10ded changed the title (DRAFT) Vault Changes Commit set 1 Add support for deposit of non PNL collaterals Sep 15, 2025
@ishay-starkware ishay-starkware force-pushed the dev branch 3 times, most recently from 28081b3 to 11211df Compare September 16, 2025 08:00
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.

1 participant