Split Test Case #164
Open
Split Test Case #164
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Adds compBorrowSpeeds and compSupplySpeeds to ComptrollerV6Storage - Change _setCompSpeed to _setCompSpeeds - one function call to update distribution speeds for multiple markets - Change signature of setCompSpeedInternal - compSpeed split up into two parts: compSupplySpeed and compBorrowSpeed
…compSupplySpeeds and compBorrowSpeeds
…ompBorrowIndex - compSupplySpeeds and compBorrowSpeeds were flipped
- These tests have been manually verified to fail when they are expected to fail => they work well
- Supply state and borrow state now initialized when a market is added, ensuring market state indices are always set
- Now that market state indices are non lazily set, we can simply update market states when updating COMP speeds
- Adds changes from PR #173 - Second mint with comp accrued: reduced by 695 - Claim comp: reduced by 1268
- Old scen test used ComptrollerG3 - Removed test for removed function which never actually worked
- We need to call the upgrade function using the delegator address to use the storage at that contract address
- Asserts that the upgrade proposal passes - Asserts that old comp speeds are deleted - Asserts that new comp speeds are the same as they were previously - Asserts that market state indices are the same post upgrade, with 0 values initialized to 1e36 - Asserts that the 2 comp speed bugs were fixed - Asserts that the new _setCompSpeeds function works - Asserts that comp rewards accrual works
…l into dynamic-comp-rewards-distribution-2
- Stores blockNumber as uint32 for safety
- Ensures supply and borrow states properly initialized - Ensures COMP is accrued correctly when COMP rewards are added (after market activation), removed, then added again - This test covers the COMP speed bug identified on August 09, 2021
- The logic didn't flow properly
- Underflow occurs when calling distributeBorrowerComp on markets whose borrowState.index didn't start at 1e36. - The COMP rewards on those markets work fine, so we just need to make a fix for people borrowing cMKR, cAAVE, and the other new markets whose indices haven't been set yet, which this commit does.
- The change to distributeBorrowerComp to account for borrowers with uninitialized state indices increased the gas costs unfortunately
- Ensures new COMP speeds apply to both prior borrowers+suppliers and later borrowers+suppliers correctly
- Ensures new COMP speeds apply to both prior borrowers+suppliers and later borrowers+suppliers correctly w/ uninitialized prior borrower/supplier state indices
- Oof, the necessary change to distributeSupplierComp increased the gas cost of claim - Lastly, gm
- The failing test of the prior commit is now passing
kevincheng96
reviewed
Oct 1, 2021
- There are COMP accrual values we need to fix before we can fully allow this - Allows COMP to be transferred to only the users which haven't interacted with the problematic markets
hayesgm
force-pushed
the
cob/split-speed-scen
branch
from
54855e4
to
880c1b8
Compare
…l into dynamic-comp-rewards-distribution-2
- Ensures proposal 64 does what it's expected to do - Please use the current mainnet configurations for this to run properly
This patch simply adds a test-case to show how some accounts interact with the current Comptroller.
hayesgm
force-pushed
the
cob/split-speed-scen
branch
from
880c1b8
to
002fd08
Compare
|
Note: this patch now adds a check on #165 with a new scenario that tests how the upgrade would affect a variety of accounts. |
jflatow
reviewed
Oct 4, 2021
| Assert Equal (Erc20 COMP TokenBalance Geoff) 2000 | ||
|
|
||
| -- Start with G8, upgrade to G9 (bug), then upgrade to latest | ||
| Only "Claim COMP across upgrade" |
TylerEther
reviewed
Oct 4, 2021
| "seventh": 6 | ||
| "seventh": 6, | ||
|
|
||
| "tyler": 7, |
|
Did the changes here already get included in merged changes? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.