ubiquity / ubiquibot

Putting the 'A' in 'DAO'
https://github.com/marketplace/ubiquibot
MIT License
17 stars 60 forks source link

Penalty for reopened issues #439

Closed rndquu closed 1 year ago

rndquu commented 1 year ago

There is an issue for which we had to pay twice because it was not correctly implemented for the 1st time. Here is how it happened. While proper QA eliminates such cases we should also add a penalty for a bounty hunter.

What should be done:

0x4007 commented 1 year ago

Could you propose architecture for this?

For example, at the time of new payment permit generation, we need to check:

  1. the issues that bounty hunter is assigned to (what if they un assign themselves after it's been reopened?)
  2. If any of those have been reopened
  3. The permit amount

If we communicate the debit ONLY at the time of payment permit generation, due to mismatched expectations (they see the bounty price being much higher than what they actually receive) the bounty hunter can be upset/disappointed and quit.

Is there a clearer way to manage expectations before they get to complete the second bounty?

rndquu commented 1 year ago

Could you propose architecture for this?

For example, at the time of new payment permit generation, we need to check:

  1. the issues that bounty hunter is assigned to (what if they un assign themselves after it's been reopened?)
  2. If any of those have been reopened
  3. The permit amount

If we communicate the debit ONLY at the time of payment permit generation, due to mismatched expectations (they see the bounty price being much higher than what they actually receive) the bounty hunter can be upset/disappointed and quit.

Is there a clearer way to manage expectations before they get to complete the second bounty?

I actually thought that this "penalty" data could be written to a DB. This way we don't need to parse all the issues on permit generation.

So if an issue is reopened then:

  1. Find permit comment
  2. Find the latest assignment before the permit generation
  3. Save bounty hunter's id, penalty amount, issue id
  4. On some other permit generation apply the penalty

This schema is cross-repo so it is possible to get a penalty in one repo but have it applied in another one.

Is there a clearer way to manage expectations before they get to complete the second bounty?

We can soften the penalty a little bit, for example by applying a rule "penalize not more than 50% of a bounty reward"

the bounty hunter can be upset/disappointed

And we are upset because the previously implemented issue didn't match the spec

0x4007 commented 1 year ago

This schema is cross-repo so it is possible to get a penalty in one repo but have it applied in another one.

I kind of like the idea of isolating repositories (or at least organizations) for this because what if the review board of one repository (or organization) is not competent? We would also have to figure out a way to redistribute "clawed back" payment to the partner that received unsatisfactory work. For example:

I think thats interesting to keep things moving forward actually. It will consolidate debt collection across our entire "metaverse task economy".

But I still have concerns with increasing risks for bounty hunters with working on a project with an incompetent review team.

More on this at #441


And we are upset because the previously implemented issue didn't match the spec

lol

Here's an idea for a new flow:

  1. An issue is re-opened due to unsatisfactory work.
  2. At this moment, if the bounty hunter is working on another issue, the bot will immediately post a warning comment. For example:
    • "@user a prior task you worked on, #9876, has been re-opened. Please be sure to review its conversation and implement any necessary fixes. Until that issue is closed again, a penalty of that issue amount will be included in your next payment."
rndquu commented 1 year ago

I kind of like the idea of isolating repositories (or at least organizations) for this because what if the review board of one repository (or organization) is not competent?

Ok, so we need to keep a penalty in boundaries of a single repo.

At this moment, if the bounty hunter is working on another issue, the bot will immediately post a warning comment

We could post this penalty warning message in the reopened issue. So if issue is reopened then post smth like:

0x4007 commented 1 year ago

"@user please be sure to review this conversation and implement any necessary fixes. Unless this is closed as completed, its payment of XXX USD will be debited from your next bounty."

rndquu commented 1 year ago

"@user please be sure to review this conversation and implement any necessary fixes. Unless this is closed as completed, its payment of XXX USD will be debited from your next bounty."

Updated description

0x4007 commented 1 year ago

Notes from research call

rndquu commented 1 year ago

If the second bounty is smaller than the first bounty we won't be made whole by the way

Then we will be made whole on the next bounty

This approach is "optimistic" but if we do "pessimistic" then we can automatically issue only e.g. half the bounty up front and if its not re-opened within a e.g. week then issue the second half payment permit.

This complicates payouts because we would somehow need to maintain 2 unique nonces per a single issue. Anyway proper QA eliminates most of poor PRs so I would not split payouts.

Venoox commented 1 year ago

/start

ubiquibot[bot] commented 1 year ago

@Venoox The time limit for this bounty is on Sat, 01 Jul 2023 11:23:09 GMT

Your currently set address is: 0x999cc482d3b04dd3dF733411687341906989Ec5B please use /wallet 0x4FDE...BA18 if you want to update it.

Venoox commented 1 year ago

What about a situation where an issue is reopened but in the mean time the bounty hunter completes another issue and gets a penalty but after that fixes the reopened issue.

I guess the warning solves this to some extent by warning the user not to finish other bounties before fixing the reopened issue.

rndquu commented 1 year ago

where an issue is reopened but in the mean time the bounty hunter completes another issue and gets a penalty but after that fixes the reopened issue

Good catch. So if a bounty hunter solves another issue and the penalty is applied then there is no incentive for a bounty hunter to solve a reopened issue.

Perhaps we should apply a progressive penalty rate (like -10% reward for each reopened issue) on all solved bounties in the future until all reopened issues are solved.

@pavlovcik what do you think?

Venoox commented 1 year ago

Also what if the payment token has changed? How do we calculate the penalty on new permit generation with new token?

rndquu commented 1 year ago

Also what if the payment token has changed? How do we calculate the penalty on new permit generation with new token?

I guess payment tokens are always(?) USD pegged so it doesn't really matter which token to use for penalty calculation, DAI or WXDAI.

Venoox commented 1 year ago

So the penalty should be written in database and restricted to one repo?

rndquu commented 1 year ago

So the penalty should be written in database

yes, unless there is a way to save it somehow in github comments

restricted to one repo?

yes

0x4007 commented 1 year ago

where an issue is reopened but in the mean time the bounty hunter completes another issue and gets a penalty but after that fixes the reopened issue

Good catch. So if a bounty hunter solves another issue and the penalty is applied then there is no incentive for a bounty hunter to solve a reopened issue.

Economically, I think that the partner is made whole (they are paid back for non-performance) so it doesn't matter. Let's continue with the original specification.

Also what if the payment token has changed? How do we calculate the penalty on new permit generation with new token?

This is a great observation. I guess to make things simple we should only enable support if its the same payment token. There is a real possibility that partner projects use volatile assets for payment (such as their own governance tokens, or Ether)

In the future we can consider tapping into some off-chain oracles. If it is on-chain then multichain support for payments (we currently support Ethereum Mainnet and Gnosis Chain as an example) will be difficult. Off-chain could be exploited more easily so we would need to think critically on a trusted data source (perhaps a major CEX like Binance or Coinbase?)

So the penalty should be written in database and restricted to one repo?

I have mixed feelings because there is a proposal to evolve this idea. To keep this conversation simple, let's restrict to a single repository.

However, for context, here is the proposal and its significance:

If we implement #441 then it is a far more powerful feature to integrate the entire bounty/task network together across all organizations and repositories. This would require cross-repository memory/support, so database will probably be the best way to handle this (unless there is a way to save it somehow in GitHub comments)

ubiquibot[bot] commented 1 year ago

@Venoox - Releasing the bounty back to dev pool because the allocated duration already ended! Last activity time: Sun Jul 16 2023 19:44:35 GMT+0000 (Coordinated Universal Time)

ubiquibot[bot] commented 1 year ago

@Venoox The time limit for this bounty is on Mon, 31 Jul 2023 13:38:56 GMT

0x4007 commented 1 year ago

The delays are on us. We should help get @Venoox work merged in asap can you guys help @ubiquity/development

ubiquibot[bot] commented 1 year ago

@Venoox - Releasing the bounty back to dev pool because the allocated duration already ended! Last activity time: Tue Jul 04 2023 16:37:37 GMT+0000 (Coordinated Universal Time)

ubiquibot[bot] commented 1 year ago

[ CLAIM 450 WXDAI ]

0x999cc482...06989Ec5B

If you enjoy the DevPool experience, please follow Ubiquity on GitHub and star this repo to show your support. It helps a lot!
0x4007 commented 1 year ago

@ubiquity/software-development has anybody seen this debt system work in production? I have not once seen it.

rndquu commented 1 year ago

@ubiquity/software-development has anybody seen this debt system work in production? I have not once seen it.

I haven't seen such cases yet. Perhaps that's because the bot's core team is fixing things all the time without reopening issues because it is simpler to fix the bug themselves compared to finding the root cause PR and waiting for a fix from a bounty hunter.

0x4007 commented 1 year ago

Issues can certainly be opened retroactively. e.g. during when a pull request is opened from somebody else fixing the issue. I've done it a few times.

I suspect that the debt system doesn't work because in our current implementation: even if worker A had their issue re-opened, and B fixed it; A should still have debt for their next bounty.

It seems that some major steps work:

  1. The bot comments that a debt will be placed on the next bounty
  2. I've seen the debts table in the database

I suspect that when creating new payouts, it does not actually check the debts table.

0x4007 commented 1 year ago
0xcodercrane commented 1 year ago

@ubiquity/software-development has anybody seen this debt system work in production? I have not once seen it.

yes during the test in ubiquibot/production