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.
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(PRT): Add
Prt
andPrtFeeSplitExtension
#176feat(PRT): Add
Prt
andPrtFeeSplitExtension
#176Changes from 10 commits
ffa3671
d30288a
2b589d2
4e6d20b
fecfa95
fe30c96
fc4b6aa
09abd4f
a0c9bcc
df8ce65
ca1a5e8
0996cfa
66abe3f
7169de6
515e783
932489a
e7a8a8c
67bc64a
8015c24
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Note that in the newer version of
SnapshotStakingPool.sol
the public variable is calleddistributor
and notfeeSplitExtension
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.
Should we also check to make sure the Staking Pool's
stakeToken
matches the address stored inprt
?In the constructor we check if
prt.setToken()
is correct, but I don't think that's effective unless we also check that the StakingPool uses that same prt.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.
added additional checks to match the
SnapshotStakingPool
here515e783
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 we should add a line for
require(address(prtStakingPool) != address(0), "PRT Staking Pool not set")
, as a reminder that this function can be called before the prtStakingPool is defined?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.
added here
e7a8a8c
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.
Does this function have to be mutual upgrade? To me it feels like this is something the operator should be responsible over. Also, the Leverage Strategy extensions
updateCallerStatus
is onlyOperator rather than mutual upgrade.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.
updated to
onlyOperator
here8015c24
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.
Same comment as above for
setAccruersStatus
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.
updated to
onlyOperator
here8015c24
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.
Noting the newer version of SnapshotStakingPool uses the variable name
distributor
instead offeeSplitExtension
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.
fixed here
515e783