COYN balance showing up incorrectly

[harshita-srivastava-yral]

Hi team,

Getting multiple user complaints that COYN balance in Wallet is reflecting incorrectly. Can we please urgently look into this.

Attaching screenshot for reference

[harshita-srivastava-yral]

@abeeshake-yral Can you please share the findings here?

[abhishek-tripathi-yral]

These are the panics from the chrome console when loading the wallet page.

[harshita-srivastava-yral]

• Not happening now and we are unable to reproduce it either on mobile or on web.

[harshita-srivastava-yral]

This is reproducible. This is happening when user doesn't have sufficient COYN balance but keeps voting. Steps to reproduct: Keep voting on the videos without waiting for result, see your wallet balance. @komal-rs @abeeshake-yral

[komal-sai-yral]

This bug is from canister code

[abhishek-tripathi-yral]

Can confirm the reproduction of the issue as suggested by @harshitasrivastav28 !

[abhishek-tripathi-yral]

Could this be a concurrency issue @komal-rs ? This happens when the user keeps placing votes and moves on without waiting for the 'bet to be placed'. In other words, the 'spinner' on the frontend keeps spinning and the user moves on.

If I do this enough number of times, such that placed bet amount > my coin balance => the random number shows up.

[komal-sai-yral]

yes very likely a concurency issue

[abhishek-tripathi-yral]

do we go the 'mutex' route for placing bets then?

we do have 'insufficient balance' check in place here

this means that all the bets are passing the validation but the time taken between inter canister calls makes it a concurrency issue. because of the calls to receive_bet_from_bet_makers_canister

[komal-sai-yral]

So we need to understand concurrency in canisters. I know it is single threaded but maybe it there is context switch when doing API calls, etc

[abhishek-tripathi-yral]

token_event_balance

graph LR
    A[add_post_v2] -- fast operation --> B(validate_incoming_bet)
    B -- slow operation (bet1) --> C(receive_bet_from_makers_canister)
    B -- slow operation (bet2) --> D(receive_bet_from_makers_canister)
    C --> E(token_event_balance)
    D --> E

[abhishek-tripathi-yral]

so, either we deduct the amount (as a reserve) before the receive_bet_from_makers_canister or use some form of mutex / semaphore (ongoing_bet = true).

but then, it would also need something like lock data structure which can retry itself until successful. Reasoning for this is:

1. bet_1 is ongoing => holds the 'lock'
2. bet_2 comes => finishes first => tried to update token_balance but cannot since 'lock' is held by bet_1 => bet_2 needs to retry after a short backoff.

To me, it seems like a spin-lock is ideal for this use case. Feel free to suggest a simpler approach

@komal-rs

[komal-sai-yral]

Best practices from ICP regarding this issue - https://internetcomputer.org/docs/current/developer-docs/security/security-best-practices/inter-canister-calls#recommendation-1

[harshita-srivastava-yral]

• To quantify the impact of the bug, can we add it in user attributes in big query to get an understand of COYN balance

[abhishek-tripathi-yral]

• bigquery event - add user balance to event
• how many users have >1bn balance? how many were affected?
• approach = validate_bet => reduce the token balance.

[Natasha-GB]

• Local testing in progress
• ETA by today eod