Skip to content
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

Adjust tribute refunding condition #146

Merged
merged 3 commits into from
Oct 15, 2024

Conversation

dusan-maksimovic
Copy link
Contributor

Closes #140

This PR does the following:

  • Tributes can be claimed only if the proposal was among the top N and it received at least the specified percentage threshold of the total voting power.
  • Tributes can be refunded only if the proposal wasn't among the top N or it was among the top N but it received less than the specified percentage threshold of the total voting power.
  • Percentage threshold is specified through the instantiation message. For the deployed contracts, a proper migration is added that allows this parameter to be specifed.

Copy link
Member

@p-offtermatt p-offtermatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think we should extract a method from refund_tribute and claim_tribute.
These two going out of sync is the kind of thing that will cause bugs in the future.

How about making it a function "TributeIsRefundable() -> (bool, str)", where the string is a message that explains the "why" and that can be used in error returns.
For example, if the proposal is in the top N and has the percentage, the return is:
false, "proposal is in the top N and has at least x% of the votes".
if its not in the top N:
true, "proposal is not in the top N"

if it does not have the percentage threshold:
true, "proposal does not have at least x% of the votes"

Then the message can be used to inform the error return. so for claiming, if "IsRefundable" returns true, it would go:
"Cannot claim tribute:" + message, so e.g. it would look like
"Cannot claim tribute: proposal is not in the top N"

@dusan-maksimovic dusan-maksimovic merged commit 5df5e47 into main Oct 15, 2024
5 checks passed
@dusan-maksimovic dusan-maksimovic deleted the dusan/adjust-refunding-condition branch October 15, 2024 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adjust refunding condition
2 participants