-
Notifications
You must be signed in to change notification settings - Fork 13
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
Refactored interest auction to use backstop token donations #188
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation looks good to me. Changes are pretty minor, only drawback is that liquidators will be filling auctions with backstop tokens, but it's worth it IMO to prevent the gulp_usdc
sandwich attack vector.
Leaving as request changes until test updates are complete.
test-suites/src/test_fixture.rs
Outdated
let (aqua_id, aqua_client) = create_token(&e, &bombadil, 7, "AQUA"); | ||
let (test1_id, test1_client) = create_token(&e, &bombadil, 7, "TEST1"); | ||
let (test2_id, test2_client) = create_token(&e, &bombadil, 7, "TEST2"); | ||
let (test3_id, test3_client) = create_token(&e, &bombadil, 7, "TEST3"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assuming these were part of exploratory testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bumping this. Can we clean the test fixture up?
- clean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor test improvements and cleanup - looks good otherwise!
let res_asset_address = reserve_list.get_unchecked(i); | ||
for res_asset_address in assets.iter() { | ||
// don't store updated reserve data back to ledger. This will occur on the the auction's fill. | ||
let reserve = pool.load_reserve(e, &res_asset_address, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a test that verifies invalid reserves throws an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't notice in the test diff.
- Add negative test for set of addresses with an invalid reserve address
test-suites/src/test_fixture.rs
Outdated
let (aqua_id, aqua_client) = create_token(&e, &bombadil, 7, "AQUA"); | ||
let (test1_id, test1_client) = create_token(&e, &bombadil, 7, "TEST1"); | ||
let (test2_id, test2_client) = create_token(&e, &bombadil, 7, "TEST2"); | ||
let (test3_id, test3_client) = create_token(&e, &bombadil, 7, "TEST3"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bumping this. Can we clean the test fixture up?
- clean
Still need to fix tests