-
Notifications
You must be signed in to change notification settings - Fork 2
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
Upgraded Q -> 3 from #619 [1675724566035] #693
Labels
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
downgraded by judge
Judge downgraded the risk level of this issue
duplicate-528
satisfactory
satisfies C4 submission criteria; eligible for awards
withdrawn by judge
Special case: this finding was auto-generated by a judge and is now withdrawn; it can be ignored
Comments
kirk-baird marked the issue as duplicate of #42 |
c4-judge
added
downgraded by judge
Judge downgraded the risk level of this issue
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
withdrawn by judge
Special case: this finding was auto-generated by a judge and is now withdrawn; it can be ignored
and removed
3 (High Risk)
Assets can be stolen/lost/compromised directly
labels
Feb 10, 2023
This auto-generated issue was withdrawn by kirk-baird |
c4-judge
added
3 (High Risk)
Assets can be stolen/lost/compromised directly
and removed
downgraded by judge
Judge downgraded the risk level of this issue
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
labels
Feb 10, 2023
This previously downgraded issue has been upgraded by kirk-baird |
c4-judge
added
the
satisfactory
satisfies C4 submission criteria; eligible for awards
label
Feb 14, 2023
kirk-baird marked the issue as satisfactory |
c4-judge
added
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
downgraded by judge
Judge downgraded the risk level of this issue
and removed
3 (High Risk)
Assets can be stolen/lost/compromised directly
labels
Feb 23, 2023
kirk-baird changed the severity to 2 (Med Risk) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
downgraded by judge
Judge downgraded the risk level of this issue
duplicate-528
satisfactory
satisfies C4 submission criteria; eligible for awards
withdrawn by judge
Special case: this finding was auto-generated by a judge and is now withdrawn; it can be ignored
Judge has assessed an item in Issue #619 as 3 risk. The relevant finding follows:
The function `withdrawRemainingTokens can be changed in a safer way to handle the withdraw from the owner and the protocol fee as well. This prevent risks allocated with the protocol fees.
By the docs this function is called in two different scenarios, if a quest is full and receipt redeemers equals the max amount of total participants allowed in the quest - only withdrawFee is called. If a quest doesn't hit the max total participants, first the owner calls the function withdrawRemainingTokens to withdraw the remaining tokens and then the fee should be paid with the function withdrawFee.
Overall the best solution of this problem is that the function withdrawRemainingTokens, both does the withdrawing part to the owner and pays the fee to the protocol as well. This is considered the safest way:
First - if the receipt redeemers are below the totalParticipants, can withdraw the remaining tokens and pay the fee at the same time, second if the quest is full and receipt redemeers hits the total amount of people allowed, only the fee will be paid to the protocol and will skip the withdraw remaining rewards part.
function withdrawRemainingTokens(address to_) public override onlyOwner {
super.withdrawRemainingTokens(to_);
The text was updated successfully, but these errors were encountered: