Potential synchrony issue of official DFINITY sample dex example, Potential lost funds?

I was looking at the sample dex examples/main.mo at master · dfinity/examples · GitHub offered by dfinity and was wondering whether there was a potential security hole in the way they do deposits. I noticed that after the subaccount receives ICP and deposits it to the main account of the canister, it is assumed that when the recording of the token and principal in the book is assumed to have gone through successfully. If you look in the code, they don’t do any resending back of the tokens to the principal if the book fails to record the transaction. So my question is, is there any potential scenario where the book fails to record the deposit when the subaccount successfully sends the icp besides if the hashmap was already full.
I do understand you could do some kind of block checking as an alternative which would allevaite this issue but the point is this was an official dfinity example and most likely audited so I was curious if the way they do it in the example is sufficient for ensuring the book recorded the balance for every single accepted transfer from subaccount to main account with no exceptions (besides a full canister which is easily alleviated). Anyone know?

    private func depositIcp(caller: Principal): async T.DepositReceipt{         
     // Calculate target subaccount
    // NOTE: Should this be hashed first instead?
    let source_account = Account.accountIdentifier(Principal.fromActor(this), Account.principalToSubaccount(caller));

    // Check ledger for value
    let balance = await Ledger.account_balance({ account = source_account });

    // Transfer to default subaccount
    let icp_receipt = if (Nat64.toNat(balance.e8s) > icp_fee) {
        await Ledger.transfer({
            memo: Nat64    = 0;
            from_subaccount = ?Account.principalToSubaccount(caller);
            to = Account.accountIdentifier(Principal.fromActor(this), Account.defaultSubaccount());
            amount = { e8s = balance.e8s - Nat64.fromNat(icp_fee)};
            fee = { e8s = Nat64.fromNat(icp_fee) };
            created_at_time = ?{ timestamp_nanos = Nat64.fromNat(Int.abs(Time.now())) };
        })
    } else {
        return #Err(#BalanceLow);
    };

    switch icp_receipt {
        case ( #Err _) {
            return #Err(#TransferFailure);
        };
        case _ {};
    };
    let available = { e8s : Nat = Nat64.toNat(balance.e8s) - icp_fee };

    // keep track of deposited ICP
    book.addTokens(caller,ledger,available.e8s);

    // Return result
    #Ok(available.e8s)
};
1 Like

Dear Iceypee,

The book is implemented as an hashmap and the deposit simply consists in putting some data in this map. This call should therefore not fail.