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

Restrict assignment on lack of funds #264

Open
rndquu opened this issue Apr 13, 2023 · 7 comments
Open

Restrict assignment on lack of funds #264

rndquu opened this issue Apr 13, 2023 · 7 comments

Comments

@rndquu
Copy link
Member

rndquu commented Apr 13, 2023

Depends on #262

We should restrict an issue assignment if there are not enough funds to cover a bounty payout.

When a bounty hunter posts /assign the bot should:

  1. Get current wallet balance
  2. Find sum of payouts of all opened assigned issues
  3. Check that bounty payout >= wallet balance - sum of payouts of opened assigned issues
  4. If there are not enough funds then print an error like "Not enough funds in the wallet to cover the bounty"

Notice that we are being optimistic here in terms that we don't cover this edge case because it is tricky and resource intensive to match an issue with an ethereum payout transaction:

  1. Wallet has 100 USD
  2. Bounty hunter A assigns and completes an issue worth 100 USD (PR is closed)
  3. Permit is generated and not yet withdrawn
  4. Bounty hunter B assigns another issue worth 100 USD (here we allow the assignment though we should restrict it)

So internally we consider all closed issues as paid out for simplicity. We have plans for a "function as a service" for matching issues with ethereum transactions. When this "function as a service" is implemented we can update the restriction logic to be more accurate.

@0x4007
Copy link
Member

0x4007 commented Apr 16, 2023

tricky and resource intensive to match an issue with an ethereum payout transaction:

I still think there is potential for encoding call data inside of the transaction for analytics and auditing purposes. This can probably be done on pay.ubq.fi claim transaction initializer in the UI code?

@rndquu
Copy link
Member Author

rndquu commented Apr 16, 2023

tricky and resource intensive to match an issue with an ethereum payout transaction:

I still think there is potential for encoding call data inside of the transaction for analytics and auditing purposes. This can probably be done on pay.ubq.fi claim transaction initializer in the UI code?

Without forking the original permit2 contract or creating our own "proxy analytics" contract (which would save github's issue id, org and repo names and then would execute a payout) we can only pass a github issue id. But we also need org and repo names for analytics.

@0x4007
Copy link
Member

0x4007 commented Apr 16, 2023

So there isn't a way to encode an arbitrary string inside of the calldata?

For example, when people send messages to each other on chain they commonly call transfer with an amount of 0 but then encode their plain English message in the transaction.

I propose that we do the same and come up with a simple encoding method, like separating each field with a comma etc.

@rndquu
Copy link
Member Author

rndquu commented Apr 17, 2023

So there isn't a way to encode an arbitrary string inside of the calldata?

For example, when people send messages to each other on chain they commonly call transfer with an amount of 0 but then encode their plain English message in the transaction.

I propose that we do the same and come up with a simple encoding method, like separating each field with a comma etc.

pls check ubiquity/audit.ubq.fi#12

@0x4007
Copy link
Member

0x4007 commented Apr 24, 2023

I propose we change this into a warning logged by the bot in a comment:

e.g. The funding wallet currently does not have enough money to pay for this bounty. Treasurers should top up the funding source soon. If you would like to proceed with the bounty please comment /assign force

and have bounty hunters override the warning (and assign themselves) with a new command. This will allow more optimistic bounty hunters to have their expectations managed and continue pushing the project forward preventing unnecessary slowdowns.

@rndquu
Copy link
Member Author

rndquu commented Apr 25, 2023

I propose we change this into a warning logged by the bot in a comment:

e.g. The funding wallet currently does not have enough money to pay for this bounty. Treasurers should top up the funding source soon. If you would like to proceed with the bounty please comment /assign force

and have bounty hunters override the warning (and assign themselves) with a new command. This will allow more optimistic bounty hunters to have their expectations managed and continue pushing the project forward preventing unnecessary slowdowns.

So for a bounty hunter the flow is the following:

  1. Bounty hunter comments /assign
  2. The bot comments with "Treasurers should top up the funding source soon. If you would like to proceed with the bounty please comment /assign force"
  3. Bounty hunter comments /assign force, then he is assigned to the issue

@0x4007
Copy link
Member

0x4007 commented Apr 25, 2023

@rndquu that flow appears to be correct according to my proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants