Skip to content

Update comment for pending_balance_hi #364

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

manxiaqu
Copy link

@manxiaqu manxiaqu commented Apr 9, 2025

The comment of pending_balance_hi seems wrong, It's actual the high 32 bits of the pending balance.

The comment  of pending_balance_hi seems wrong, It's actual the high 32 bits of the pending balance.
@manxiaqu
Copy link
Author

manxiaqu commented Apr 9, 2025

the total available_balance is 48 bits encrypted,available_balance = available_balance + (pending_balance_lo + pending_balance_hi * 2^16) ,So the pending_balance_hi should be 32 bits of balance, and the pending_balance_lo is 16b bits of balance.

Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this -- the comment here is actually correct though.

We limit the transfer amount to the low 16 bits and high 32 bits (see

/// Any deposit or transfer amount must be less than `2^48`
pub const MAXIMUM_DEPOSIT_TRANSFER_AMOUNT: u64 = (u16::MAX as u64) + (1 << 16) * (u32::MAX as u64);
)

The pending amount, however is split between the low 16 bits and high 48 bits. This allows for users to receive many transfers of a maximum of 2^48 without performing ApplyPendingBalance in between

@manxiaqu
Copy link
Author

Thanks for your reply. I still have some confusion.
The deposit amount is 48 bits and split in [32bits, 16bits]. And the 32 bits is encrypted in pending_balance_hi, the 16 bits is encrypted in pending_balance_lo.

And also the max encrypted amount of pending_balance_hi or pending_balance_lo is 32 bits long.


if the pending_balance_hi is high 48 bits. Then it will overflow, because the decrypt function can only get amount in 32 bits long, but the pending_balance_hi is 48 bits long.

Is there something wrong with my understanding?

@joncinque
Copy link
Contributor

I double-checked with our cryptographer because you stumped me for a bit!

So the comment is correct currently, and the hi bits do actually contain the top 48 bits.

Transfers are limited to 48 bits total (16 low and 32 hi). If someone transfers 2^48 tokens into your account twice, then pending_balance_hi will contain a value larger than 2^32, which is totally fine.

However, the decryption function that you noticed will fail since the value is larger than 2^32. In that case, you'll need to go through the transfer transactions to properly decrypt the value.

Our cryptographer noted that it's possible to decrypt values up to around 2^41 in a reasonable amount of time, so he'll add more decryption functions in case pending_balance_hi is greater than 2^32.

In the meantime, I propose to close the PR, but I really appreciate you bringing up this issue.

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.

2 participants