-
Notifications
You must be signed in to change notification settings - Fork 17
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
fix: agToken update #205
fix: agToken update #205
Conversation
I only see format update on this PR |
82c0a45
to
1462799
Compare
} | ||
|
||
/// @inheritdoc IAgToken | ||
function setTreasury(address _treasury) external onlyTreasury { |
Check notice
Code scanning / Slither
Missing zero address validation Low
/// @notice Reference to the treasury contract which can grant minting rights | ||
address public treasury; | ||
/// @notice Boolean used to check whether the contract had been reinitialized after an upgrade | ||
bool public treasuryInitialized; |
Check warning
Code scanning / Slither
State variables that could be declared constant Warning
// ================================= REFERENCES ================================ | ||
|
||
/// @notice Reference to the `StableMaster` contract associated to agEUR | ||
address public stableMaster; |
Check warning
Code scanning / Slither
State variables that could be declared constant Warning
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.
For the tests it worth a codecov run to check that we are still at 100%
|
||
export const immutableCreate2Factory = '0x0000000000FFe8B47B3e2130213B802212439497'; | ||
|
||
export const OFTs: OFTsStructure = { |
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.
We can do a call to our sdk for that
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.
We can, only reason why I'm fine not doing it now is because when you deploy it's often a pain to have to wait for the SDK update to get all the addresses here
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 am not sure I understand your point?
d588a4e
to
2f59eec
Compare
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.
Also for the vanity logic, as we will surely have it on multiple repos, best is to have a specific repo implementing this logic (with parameters) that will be imported where needed
|
||
export const immutableCreate2Factory = '0x0000000000FFe8B47B3e2130213B802212439497'; | ||
|
||
export const OFTs: OFTsStructure = { |
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 am not sure I understand your point?
function _getQuoteAmount() internal view virtual returns (uint256) { | ||
return 10 ** 18; | ||
} |
Check warning
Code scanning / Slither
Dead-code Warning
import { parseAmount } from '../../utils/bignumber'; | ||
|
||
const flashLoanParams = { | ||
// 3m at the moment, should not be too big with respect to the total agEUR in circulation |
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.
Maybe lower with stUSD as we don't know the growth yet
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.
Adjusted
|
||
export const interestRate5 = BigNumber.from('1547125982881425408'); | ||
|
||
export const vaultManagers = { |
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.
Why is it in this repo and not the sdk one?
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.
Because we removed the parameters from the sdk
894f7a7
to
2f9e9c9
Compare
No description provided.