Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check account exists in vault.state before accessing it. #304

Merged
merged 2 commits into from
Nov 11, 2024

Conversation

0xhipo
Copy link
Contributor

@0xhipo 0xhipo commented Nov 10, 2024

Fixed #303

Description:
This pull request addresses the issue where the locked_balance function aborts when called on a fresh balance_manager that has not interacted with the pool. The solution involves adding a check for locked_balance before placing an order.

Changes Made:

  • Updated pool::locked_balance, check account exists before accessing.
  • Updated deepbook::master_tests::test_locked_balance to include check_locked_balance<SUI, USDC>(ALICE, pool1_id, alice_balance_manager_id, &alice_locked_balance, &mut test); before placing an order.

Testing:

  • The updated test case now passes, confirming the fix.
  • Verified that the computational load increase is minimal and acceptable.

base_quantity = base_quantity + settled_balances.base();
quote_quantity = quote_quantity + settled_balances.quote();
deep_quantity = deep_quantity + settled_balances.deep();
if (self.state.account_exists(balance_manager.id())) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we put this if statement in the beginning of the function, and keep the remaining code the same?

/// Returns the locked balance for the balance_manager in the pool.
/// Locked balance = open orders + settled balances.
/// Returns (base_quantity, quote_quantity, deep_quantity).
public fun locked_balance<BaseAsset, QuoteAsset>(
    self: &Pool<BaseAsset, QuoteAsset>,
    balance_manager: &BalanceManager,
): (u64, u64, u64) {
    let account_orders = self.get_account_order_details(balance_manager);
    let self = self.load_inner();
    if (!self.state.account_exists(balance_manager.id())) {
        return (0, 0, 0)
    };

    let mut base_quantity = 0;
    let mut quote_quantity = 0;
    let mut deep_quantity = 0;

    account_orders.do_ref!(|order| {
        let maker_fee = self.state.history().historic_maker_fee(order.epoch());
        let (base, quote, deep) = order.locked_balance(maker_fee);
        base_quantity = base_quantity + base;
        quote_quantity = quote_quantity + quote;
        deep_quantity = deep_quantity + deep;
    });

    let settled_balances = self
        .state
        .account(balance_manager.id())
        .settled_balances();
    base_quantity = base_quantity + settled_balances.base();
    quote_quantity = quote_quantity + settled_balances.quote();
    deep_quantity = deep_quantity + settled_balances.deep();

    (base_quantity, quote_quantity, deep_quantity)
}

@0xaslan 0xaslan merged commit 7996492 into MystenLabs:main Nov 11, 2024
1 check passed
tonylee08 added a commit that referenced this pull request Feb 25, 2025
* Create Pool Actions (#295)

* create pools

* update default

* Versioning Actions (#292)

* Redeploy Mainnet Pool using new tick size (#296)

* update tick size

* min size update

* new tick size (#297)

* update sdk (#298)

* add test attribute and license (#300)

* Allow admin to change pool book params (#299)

* admin update lot size

* tests

* tests

* book change event

* timestamp update

* power of 10

* Check account exists in vault.state before accessing it. (#304)

* Check account exists in vault.state before accessing it.

* Further optimize the logic. Return at the beginning.

* Add check in state.withdraw_settled_amounts. Check account exists before trying to settle account. (#306)

* Update README.md (#301)

* Update README.md

* format Readme and remove outdated docs

* example sdk usage

* create ns pools and package updates (#307)

* Pool update (#308)

* typus pools

* update

* ausd pools (#309)

* Governance and Level 2 changes (#311)

* new deepbook Pool (#313)

* New Deepbook Pool (#314)

* new deepbook Pool

* sdk update

* fix: rename EinvalidLoanQuantity to EInvalidLoanQuantity (#312)

* Allow burning 0 DEEP tokens (#315)

* allow burning 0 DEEP tokens

* deep burned event

* emit deposit event on rebate collection (#316)

* emit balance manager creation event (#317)

* 3.1 Claim Rebates (#318)

* claim rebate for all assets

* rebate deposit events

* formatting

* Pay trading fees with input token (#319)

* emit volumes event on epoch change

* emit volumes event on epoch change

* pay trading fee with input token

* gitignore

* remove pnpm lock

* undo gitignore

* Permissionless Pool Creation (#322)

* first iteration of permissionless pool creation

* permissionless pool creation

* whitelist false for permissionless

* Fix broken link in the README.md (#320)

* Typo fix in the README.md (#321)

* Base and Quote Rebates (#323)

* rebate

* just deep rebates

* wip rebates

* rebate tests

* update functions

* cleanup

* WGIGA Pool (#324)

* giga pool action

* update sdk

* prettier

* v4

* update sdk

* Input token tests (#325)

* format tests (#326)

* Swap without DEEP price (#327)

* basic swaps

* cleanup

* cleanup

* epoch information event (#328)

* lower taker fee bound (#329)

* Improvements (#332)

* improvements

* fill variable

* update ref

* Balance manager - deposit/withdraw caps, create bm with owner (#330)

* deposit and withdraw caps

* remove pnpm lock

* comments

* fix caps

* revoke

* rename error

* fix unit test

* upgrade testnet (#335)

---------

Co-authored-by: 0xaslan <[email protected]>
Co-authored-by: Hipo <[email protected]>
Co-authored-by: zero-mover <[email protected]>
Co-authored-by: teenager-ETH <[email protected]>
Co-authored-by: CODe <[email protected]>
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.

locked_balance Function Aborts on Fresh balance_manager Interaction
2 participants