-
Notifications
You must be signed in to change notification settings - Fork 146
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
feat: remove asset holder allowlist, with additional fixes #864
Conversation
BREAKING CHANGE: Staking.setAssetHolder is removed and the assetHolders mapping is deprecated.
Signed-off-by: Tomás Migone <[email protected]>
- Add a comment about the risk of tx being confirmed late causing tokens to be burnt (TRST-L-1) - Return early to avoid emitting an event if query fees are zero - Remove comment about only valid senders being able to call collect(). BREAKING CHANGE: collect() does not emit AllocationCollected if query fees are zero
fix: improvements from Trust audit (TRST-L-1 and recommendations)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #864 +/- ##
==========================================
+ Coverage 92.60% 92.69% +0.08%
==========================================
Files 47 47
Lines 2368 2394 +26
Branches 432 435 +3
==========================================
+ Hits 2193 2219 +26
Misses 175 175
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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 nits.
3fe1edb
to
3882b35
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.
lgtm
feat: remove asset holder allowlist, with additional fixes
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
* @param _tokensIn Amount of tokens used to mint signal | ||
* @return Amount of tokens that would be recovered after minting and burning signal | ||
*/ | ||
function tokensToSignalToTokensNoTax(bytes32 _subgraphDeploymentID, uint256 _tokensIn) |
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.
lol @ that name
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.
tokensToSignalToTokensNoTaxFinalFinalVersion4.docx
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.
looks good but we are not testing the GNS round attack no?
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.
nvm, just saw the test
No description provided.