Need advise for reentrancy issue best practise

Hi devs!

Recently our team discover that our system is not implement correct due to prevent reentrancy issue.

We have one backend method update_action sequentially does:

  1. loop all the transaction of action, check it is valid to execute

  2. for each executable transaction
    2.1 update transaction to processing
    2.2 spawn timeout task

  3. If transaction have to execute by current canister
    3.1 call ledger canister to transfer fund (Async)

  4. If transaction is user’s transaction, prepare list of transaction (List A) wallet have to execute

  5. Return current action and List A for client to execute

The method update_action designed to be triggered multiple times for one action. But it can be compromise by bad actor to make reentrancy issue

So we are thinking adding a new lock mechanism

  1. Create a lock for action id, store it in stable memory

  2. loop all the transaction of action, check it is valid to execute

  3. for each executable transaction
    2.1 update transaction to processing
    2.2 spawn timeout task

  4. If transaction have to execute by current canister
    3.1 call ledger canister to transfer fund (Async)

  5. If transaction is user’s transaction, prepare list of transaction (List A) wallet have to execute

  6. Delete the lock record

  7. Return current action and List A for client to execute

From what I know, if there is one transaction
from 1 → before 3 is one message execution
and from 3 → 7 is another message execution

Question
Is that any guarantee that 3 → 7 always executed after subnet is downed?

Assuming the subnet becomes live again 3-7 will always process, but if you use bounded-wait calls then you may not know if the ledger call succeeded

1 Like

is that ic_cdk::call is unbounded wait call?

Yes, ic_cdk::call is unbounded wait.