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

Inconsistent reserve for recovery function in locker_bill.fc #3

Open
thekiba opened this issue Jun 13, 2023 · 1 comment
Open

Inconsistent reserve for recovery function in locker_bill.fc #3

thekiba opened this issue Jun 13, 2023 · 1 comment

Comments

@thekiba
Copy link

thekiba commented Jun 13, 2023

Hi,

During my review of your locker.fc and locker_bill.fc contracts, I noticed an inconsistency that might be a minor bug.

In the locker.fc contract, when making a deposit, a locker_bill.fc contract is deployed with a balance of 0.5 TON:

cell deposit_body = begin_cell()
        .store_uint(op::deposit_to_bill, 32)
        .store_coins(amount)
        .end_cell();

builder msg = begin_cell()
        .store_uint(BOUNCEABLE, 6)
        .store_slice(bill_address)
        .store_coins(ONE_TON / 2)
        .store_uint(4 + 2 + 1, 1 + 4 + 4 + 64 + 32 + 1 + 1 + 1)
        .store_ref(state_init)
        .store_ref(deposit_body);

send_raw_message(msg.end_cell(), SEND_MODE_REGULAR);

However, in the locker_bill.fc contract, the recover function reserves 1 TON for storage fees:

if (is_recover) { ;; if the user mistakenly sent a deposit not to the locker but directly to the bill - we give him the opportunity to withdraw these coins
    throw_unless(error::only_user_address, equal_slices(sender_address, user_address));

    raw_reserve(ONE_TON, 2); ;; reserve 1 ton for storage fees

    builder msg = create_msg(BOUNCEABLE, user_address, 0);
    send_raw_message(msg.end_cell(), SEND_MODE_CARRY_ALL_BALANCE);

    return ();
}

This implies that if a user mistakenly sends a deposit not directly to the locker, but to the bill, an additional 0.5 TON would get locked. As a result, the recovery function might return fewer funds than initially deposited, leading to an unexpected loss for the user.

Please review this potential issue. Thank you.

@tolya-yanot
Copy link
Member

The user who sent coins to the wrong place should be prepared for their loss.

However, we have made this method so that even in this case, the user can return the coins sent by mistake.

You are right that the user will not be able to return 0.5 TON. In order not to spend too much time on this non-priority functionality, we rounded the calculation.

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

No branches or pull requests

2 participants