feat: remove asset holder allowlist (with latest main) #855
Conversation
BREAKING CHANGE: Staking.setAssetHolder is removed and the assetHolders mapping is deprecated.
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
- 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)
pcarranzav
force-pushed
the
pcv/asset-holder-dev
branch
from
24d960d
to
04a42bc
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## dev #855 +/- ##
==========================================
- Coverage 92.60% 92.59% -0.02%
==========================================
Files 47 47
Lines 2368 2362 -6
Branches 432 430 -2
==========================================
- Hits 2193 2187 -6
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. |
pcarranzav
changed the title
| * @dev Emitted when `caller` set `assetHolder` address as `allowed` to send funds | ||
| * to staking contract. | ||
| */ | ||
| event AssetHolderUpdate(address indexed caller, address indexed assetHolder, bool allowed); |
There was a problem hiding this comment.
(GitHub won't let me comment on the correct LOC as you did not modify it but...)
The event RebateCollected uses assetHolder name for the query fees provider. We probably want to change that name for clarity, something like rebateProvider or provider, sender, wdyt?
| // Allocation must exist | ||
| AllocationState allocState = _getAllocationState(_allocationID); | ||
| require(allocState != AllocationState.Null, "!collect"); | ||
|
|
||
| // If the query fees are zero, we don't want to revert | ||
| // but we also don't need to do anything, so just return |
There was a problem hiding this comment.
This slightly changes the behavior of the function as we now are not emitting the RebatesCollected event when _tokens == 0. I can't think why this could be a problem, but wanted to make sure we consider it.
|
Replaced by #864 |
No description provided.