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

Add deposit up to limit functionality #275

Closed
wants to merge 4 commits into from

Conversation

dyodx
Copy link

@dyodx dyodx commented Jan 10, 2025

Closes #274. All feedback welcome.

A couple thoughts:

  1. Here the constraint total_deposits_amount < deposit_limit is enforced, so I currently have it going up to deposit_limit - 1.
  2. Is there any reason not to have an early return if the deposit amount == 0?

@dyodx dyodx marked this pull request as draft January 10, 2025 12:10
@dyodx dyodx marked this pull request as ready for review January 10, 2025 13:15
@jgur-psyops
Copy link
Contributor

re (2), there's no reason, we could early return for 0 deposits (or error) to save some compute.

@dyodx
Copy link
Author

dyodx commented Jan 13, 2025

re (2), there's no reason, we could early return for 0 deposits (or error) to save some compute.

done, i had to rearrange the test because when we return early, a bank account doesn't get created so there are no balances to check. So that seems fine but yea, the concern was because I just don't know what assumptions (or desirable side effects) might be being made somewhere else in this or other systems. One that I want to double check -- would we want to emit LendingAccountDepositEvent even when we return early?

@jgur-psyops
Copy link
Contributor

Consolidated into #295

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.

Deposit up to limit
2 participants