Skip to content

FLO-30: Update depositLimit calculation#292

Open
tim-barry wants to merge 7 commits intomainfrom
tim/FLO-30-depositLimit
Open

FLO-30: Update depositLimit calculation#292
tim-barry wants to merge 7 commits intomainfrom
tim/FLO-30-depositLimit

Conversation

@tim-barry
Copy link
Collaborator

Closes: #239

Description

Updates calculation of depositLimit() to be in line with the recommended calculation, i.e.: the limit is now only dependent on the current deposit capacity and the limit fraction (5% of the deposit cap). Until the deposit capacity is near-exhausted, all deposits will now have the same limit (5% of the cap), rather than the limit asymptotically shrinking.

Note that before this change, the depositLimit was never zero and the capacity was never totally consumed (though this was unintended); now the capacity may become zero.

@tim-barry tim-barry requested a review from a team as a code owner March 24, 2026 22:00
@tim-barry tim-barry changed the title Update depositLimit calculation FLO-30: Update depositLimit calculation Mar 24, 2026
Copy link
Member

@jordanschalm jordanschalm left a comment

Choose a reason for hiding this comment

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

While you're working in this code, do you mind taking a look at this span of code. We separately check depositLimit and the user's remaining deposit capacity, but these values are largely defined in terms of one another. It seems like we should only need one check, against the maximum amount the user is allowed to deposit. If you can think of a way to consolidate those two checks (or fields) into one, that would be great.

@tim-barry
Copy link
Collaborator Author

Consolidated into one check for the maximum allowable deposit and one queued deposit. Some unresolved issues around the deposit limit:

asyncUpdatePosition seems to be doing extra work in checking itself whether part of a deposit needs to be re-queued rather than relying on _depositEffectsOnly, but I'm not sure if we want to explicitly make it part of _depositEffectsOnly's contract that it queues excess because of #215 and #223 .

Separately, in _depositEffectsOnly, events are emitted for the initial value of an attempted deposit (including any queued value) rather than the acceptedamount, so since asyncUpdatePosition again calls _depositEffectsOnly, queued deposits will be counted multiple times by these events - I believe this is a bug, but am not entirely sure what exactly we want to show if a deposit is partially (or entirely) queued (see also #117).

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.

FLO-30: depositLimit() Prevents Full Exhaustion of Capacity and Creates Transaction Order Dependency

2 participants