ubiquity-os / permit-generation

A standalone module to generate permits.
1 stars 19 forks source link

Dollar permits with fees #16

Closed rndquu closed 3 months ago

rndquu commented 4 months ago

User stories:

This issue touches the following repositories / docs:

ubiquibot[bot] commented 4 months ago

@rndquu the deadline is at 2024-06-19T06:12:05.277Z

rndquu commented 4 months ago

@0x4007 There are 4 options of how to implement permit fees:

  1. Direct transfers. So when permit is generated we simply transfer (since we know partner's private key) the permit fee to the treasury address.
    • Pros: Easy to implement, partner pays for gas
    • Cons: It may seem strange from a partner's point of view that some funds are transferred from time to time
  2. Batch permits. SignatureTransfer supports setting 2 destination addresses in a permit. So when permit is generated we can set that 95% of the permit reward would go to the contributor and 5% to the treasury. The issue with this approach is that contributor will be able to set his own address instead of the treasury address and get the fee since SignatureTransferDetails object is not signed by the partner's private key.
    • Pros: Easy to implement, contributors pay for gas
    • Cons: Malicious contributor is able to collect treasury fees
  3. Custom permit2 contract. We can modify SignatureTransfer and set SignatureTransferDetails to be part of the permit signature.
    • Pros: Solid solution, contributors pay for gas
    • Cons: Complex solution, requires an audit (?)
  4. On permit generation create 2 permits: one for contributor and one for the treasury EOA. Save permits to DB and claim treasury permits using CRON job.
    • Pros: Easy to implement
    • Cons: Treasury EOA pays for gas

@gentlementlegen @web4er @molecula451 @gitcoindev @whilefoo @Keyrxng Other options are appreciated

gitcoindev commented 4 months ago

Good proposals. Number 1 seems strange for me, at least from the partner's perspective. I would reject number 2 immediately. Between number 3 and 4, 3 seems the 'proper' but more complex, if requires audit also costly. For number 4, even if treasury pays for gas, I guess the cost would be negligible for L2 permits (or simply treasury would earn 5% - gas fee). Therefore I vote for option '4', and 2nd choice is option '3'.

Keyrxng commented 4 months ago

The only other thing I can think of is just a more complex alternative of 4. Automate treasury permits through OZ Defender but it's a long way for a shortcut.

I'd opt for 4, as it's the easiest to implement and like @gitcoindev says the fee is negligible. 3 seems like overkill despite it being my 2nd choice as well.

EresDev commented 4 months ago

very well thought out.

If I am not mistaken, there is a solution in permit2 that combines the pros of your option 2 & 3 and mostly removes the cons of both. It appears permitWitnessTransferFrom allows you to lock the SignatureTransferDetails and the bounty hunter can't change the "transfer to" address. I could be wrong because I haven't been able to fully understand and try it, probably will need some time to play with it to confirm. It also has a batch variation to combine multiple transfers batch-permitWitnessTransferFrom

Personally, even if I am right, I would still not go with this option or the option 2 or 3. The ability to change SignatureTransferDetails offers flexibility as it did in offering fiat visa/mastercard as a payment option. It will also help with upcoming Gnosis Pay Integration. And it keeps the door open for other opportunities.

Option 1 & 4 are the simpler solutions and the change will not leak into other repositories. The option-4 will require a cron job setup but 1 will be easier to implement and something similar is already being considered in Automatic Transfer The cons of option-1 are not serious as the partner would be already aware of the fee. And by the time fee is deducted, partner would have received the value in return.

molecula451 commented 4 months ago
  1. Direct transfers seems an option to me, we save time and cost in implementation and maybe implement some logs stuff to the DB to handle records etc, why no? seems to be the simplest, partners should be aware of which payment receiving from which whitelist address and lastly the ideal would be

a combination of the 2-3 with some combination added from the 4

0x4007 commented 4 months ago

It will also help with upcoming Gnosis Pay Integration. And it keeps the door open for other opportunities.

It is appealing to me if we can build infrastructure that helps with more objectives.

To be honest, many of these permit2 method details are beyond me.

A suggestion is that we can do the "lower security" options, and then enforce that they do not tamper with the code by cutting off their service (kernel ignores their installation ID/organization) if they have an outstanding balance.


Direct transfers always seemed the most straightforward to me though. Perhaps we can dynamically calculate the gas fee and withdraw the amount due, minus the gas cost. However, especially on mainnet, I am unsure what we would do if gas fees exceed our profit within a transaction. In these cases my mind jumps to "roll up" balances, leveraging something like the ERC20 allowance method; but that might be an off topic conversation for another time.

rndquu commented 4 months ago

Thanks everyone.

So we can't use custom permit2 contract with signed transfer details since it breaks (at least) https://github.com/ubiquity/pay.ubq.fi/pull/226.

If we think of fees in terms of monetization for plugin developers then direct transfers from partner wallet is not ideal since: 1) This way partner pays for gas both for fees and plugin developers rewards 2) We have a headache of calculating gas costs so solvency won't be 100% 3) We have a headache of implementing a retry payout mechanic since transactions will fail from time to time

Overall it seems that right now the best option is to generate 2 permits: contributor reward and fee for the treasury.

Consider this example with 5% reward fee and 3 plugins in the chain (each eligible for 25% of the fee) that should be monetized: a) Contributor solves an issue worth 100 USD b) There are 5 permits generated in a DB: 1) Our standard permit reward for 95 USD 2) Fee permit for the treasury worth 1.25 USD 3) Plugin developer 1 permit reward worth 1.25 USD 4) Plugin developer 2 permit reward worth 1.25 USD 5) Plugin developer 3 permit reward worth 1.25 USD

So both treasury EOA and plugin developers are responsible for claiming only their own permits + both of them pay for gas themselves.

In the next "fee feature" iteration, as 0x4007 said, we could consolidate those permits and create a handy UI to claim everything in a single transaction.

0x4007 commented 4 months ago

In the next "fee feature" iteration, as 0x4007 said, we could consolidate those permits and create a handy UI to claim everything in a single transaction.

This seems tricky because we need to securely prove to the partner signer that we are "trading" previously generated permits to create a new permit D that is the sum value of fee permits A, B, and C.

I wonder if there is an opportunity here for NFTs to represent the debts. Then we can easily record from which signer (partner) owes the debt, and how much the debt is worth. Then we can have a smart contract that allows a user to deposit all the NFT debts, and withdraw the sum total all at once?


    struct Debt {
        address partner;
        uint256 amount;
        address beneficiary;
    }

Minting the NFTs would likely only be viable on a network like Gnosis Chain, but it could remove the database dependency, which is nice.


One other-slightly off topic-monetization method that should be kept in mind, and supported in the future: monthly recurring. For example, ideally, if we can enforce on chain that in order to add the bot to your organization at all, you need to pay $25 a month etc.

I'm unsure if there is open source tech for this, but being able to manage this billing method entirely on-chain will be great for our bottom line.

rndquu commented 4 months ago

monthly recurring. For example, ideally, if we can enforce on chain that in order to add the bot to your organization at all, you need to pay $25 a month etc

Since we already enforce partners to have an encrypted private key for permits we could use the address derived from that private key to check (on-chain) that the monthly fee is paid.

So for a partner the flow could be:

  1. Partner sends 25 DAI and github organization id to our smart contract
  2. Partner installs the bot
  3. On github event the kernel checks that: a) address derived from evmPrivateEncrypted bot config param has a paid subscription b) github event source organization id matches the one in the smart contract (to prevent the case when a malicious partner tries to use an encrypted PK from another partner/organization)
  4. If the fee is paid the kernel processes the request otherwise throws an error
ubiquibot[bot] commented 3 months ago
+ Evaluating results. Please wait...
ubiquibot-dev[bot] commented 3 months ago

[ 1366.86 WXDAI ]

@rndquu
Contributions Overview
View Contribution Count Reward
Issue Task 1 1200
Issue Specification 1 6.48
Issue Comment 3 99.98
Review Comment 10 60.4
Conversation Incentives
Comment Formatting Relevance Reward
User stories: - Partner should be able to set any ERC20 token as…
10.8
content:
  p:
    count: 108
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.6 6.48
@0x4007 There are 4 options of how to implement permit fees: 1. …
48.6
content:
  p:
    count: 239
    score: 1
  a:
    count: 4
    score: 1
wordValue: 0.2
formattingMultiplier: 1
0.8 38.88
Thanks everyone. So we can't use custom permit2 contract with si…
48.6
content:
  p:
    count: 243
    score: 1
wordValue: 0.2
formattingMultiplier: 1
0.9 43.74
Since we already enforce partners to have an encrypted private k…
24.8
content:
  p:
    count: 123
    score: 1
  code:
    count: 1
    score: 1
wordValue: 0.2
formattingMultiplier: 1
0.7 17.36
Depends on https://github.com/ubiquibot/conversation-rewards/pul…
0
content:
  p:
    count: 68
    score: 1
  code:
    count: 3
    score: 1
  strong:
    count: 9
    score: 0
wordValue: 0
formattingMultiplier: 0
0.3 -
Treasury must be a separate github account registered with `…
4.4
content:
  p:
    count: 10
    score: 1
  code:
    count: 1
    score: 1
wordValue: 0.2
formattingMultiplier: 2
0.4 1.76
Removed default value for all permit related env variables https…
5.6
content:
  p:
    count: 14
    score: 1
wordValue: 0.2
formattingMultiplier: 2
0.5 2.8
We do need to modify the `total` because otherwise permi…
15.2
content:
  p:
    count: 34
    score: 1
  code:
    count: 1
    score: 1
  a:
    count: 3
    score: 1
wordValue: 0.2
formattingMultiplier: 2
0.4 6.08
This plugin's codebase uses `decimal.js` for all of the …
7.2
content:
  p:
    count: 17
    score: 1
  code:
    count: 1
    score: 1
wordValue: 0.2
formattingMultiplier: 2
0.6 4.32
Permit fee is already applied at the end of all calculations. Re…
6.8
content:
  p:
    count: 16
    score: 1
  code:
    count: 1
    score: 1
wordValue: 0.2
formattingMultiplier: 2
0.5 3.4
@0x4007 This PR generates a separate fee permit for the treasury…
31.2
content:
  p:
    count: 77
    score: 1
  code:
    count: 1
    score: 1
wordValue: 0.2
formattingMultiplier: 2
0.6 18.72
@gentlementlegen Help There are 5 failing tests in [this](https…
80
content:
  p:
    count: 125
    score: 1
  a:
    count: 2
    score: 1
  code:
    count: 73
    score: 1
wordValue: 0.2
formattingMultiplier: 2
0.2 16
As far as I understand it does have access to secrets but only t…
12
content:
  p:
    count: 30
    score: 1
wordValue: 0.2
formattingMultiplier: 2
0.4 4.8
You mean adding those properties to the `result` [data s…
8.4
content:
  p:
    count: 18
    score: 1
  code:
    count: 1
    score: 1
  a:
    count: 2
    score: 1
wordValue: 0.2
formattingMultiplier: 2
0.3 2.52

[ 2.31 WXDAI ]

@gitcoindev
Contributions Overview
View Contribution Count Reward
Issue Comment 1 2.31
Conversation Incentives
Comment Formatting Relevance Reward
Good proposals. Number 1 seems strange for me, at least from the…
7.7
content:
  p:
    count: 77
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.3 2.31

[ 1.8 WXDAI ]

@Keyrxng
Contributions Overview
View Contribution Count Reward
Issue Comment 1 1.8
Conversation Incentives
Comment Formatting Relevance Reward
The only other thing I can think of is just a more complex alter…
6
content:
  p:
    count: 60
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.3 1.8

[ 15.89 WXDAI ]

@EresDev
Contributions Overview
View Contribution Count Reward
Issue Comment 1 15.89
Conversation Incentives
Comment Formatting Relevance Reward
very well thought out. If I am not mistaken, there is a solutio…
22.7
content:
  p:
    count: 218
    score: 1
  a:
    count: 7
    score: 1
  code:
    count: 2
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.7 15.89

[ 9.6 WXDAI ]

@molecula451
Contributions Overview
View Contribution Count Reward
Issue Comment 1 9.6
Conversation Incentives
Comment Formatting Relevance Reward
1. Direct transfers seems an option to me, we save time and cost…
12.8
content:
  ol:
    count: 64
    score: 1
  li:
    count: 64
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.75 9.6

[ 30.53 WXDAI ]

@0x4007
Contributions Overview
View Contribution Count Reward
Issue Comment 2 29.1
Review Comment 3 1.43
Conversation Incentives
Comment Formatting Relevance Reward
It is appealing to me if we can build infrastructure that helps …
14.1
content:
  p:
    count: 141
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.6 8.46
This seems tricky because we need to securely prove to the partn…
25.8
content:
  p:
    count: 243
    score: 1
  code:
    count: 15
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.8 20.64
What is this?
0.3
content:
  p:
    count: 3
    score: 1
wordValue: 0.1
formattingMultiplier: 1
- -
Consider using a big number library for any monetary calculation…
2.1
content:
  p:
    count: 21
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.3 0.63
Oh I see. If it simplifies the code then yeah it probably is the…
1.6
content:
  p:
    count: 16
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.5 0.8

[ 6 WXDAI ]

@whilefoo
Contributions Overview
View Contribution Count Reward
Review Comment 1 6
Conversation Incentives
Comment Formatting Relevance Reward
this looks good but I have another idea: how about we add anoth…
7.5
content:
  p:
    count: 72
    score: 1
  code:
    count: 3
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.8 6

[ 0.4 WXDAI ]

@gentlementlegen
Contributions Overview
View Contribution Count Reward
Review Comment 5 0.4
Conversation Incentives
Comment Formatting Relevance Reward
Any reason to only default this one?
0.7
content:
  p:
    count: 7
    score: 1
wordValue: 0.1
formattingMultiplier: 1
- -
I see a lot of `toFixed` in here, wouldn't it be interes…
3
content:
  p:
    count: 29
    score: 1
  code:
    count: 1
    score: 1
wordValue: 0.1
formattingMultiplier: 1
- -
You should not need to modify the `total` as it gets agg…
2
content:
  p:
    count: 19
    score: 1
  code:
    count: 1
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.1 0.2
Ha good catch. Might want to change the workflow later on maybe …
2
content:
  p:
    count: 20
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.1 0.2
@rndquu When the Jest runs, since it's a fork, it doesn't have a…
3.7
content:
  p:
    count: 36
    score: 1
  code:
    count: 1
    score: 1
wordValue: 0.1
formattingMultiplier: 1
- -
ubiquibot[bot] commented 3 months ago

[ 48.3 WXDAI ]

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment248.3
Conversation Incentives
CommentFormattingRelevanceReward
> It will also help with upcoming Gnosis Pay Integration. And...
15.1
hr:
  count: 1
  score: "1"
  words: 0
0.75515.1
> In the next "fee feature" iteration, as 0x4007 said, we cou...
33.2
li:
  count: 3
  score: "3"
  words: 36
code:
  count: 5
  score: "5"
  words: 4
hr:
  count: 1
  score: "1"
  words: 0
0.6533.2

[ 28.2 WXDAI ]

@EresDev
Contributions Overview
ViewContributionCountReward
IssueComment128.2
Conversation Incentives
CommentFormattingRelevanceReward
very well thought out. If I am not mistaken, there is a solu...
28.2
a:
  count: 4
  score: "4"
  words: 8
code:
  count: 2
  score: "2"
  words: 2
0.83528.2

[ 7.5 WXDAI ]

@molecula451
Contributions Overview
ViewContributionCountReward
IssueComment17.5
Conversation Incentives
CommentFormattingRelevanceReward
1. Direct transfers seems an option to me, we save time and cost...
7.5
li:
  count: 1
  score: "1"
  words: 52
0.727.5

[ 7.7 WXDAI ]

@gitcoindev
Contributions Overview
ViewContributionCountReward
IssueComment17.7
Conversation Incentives
CommentFormattingRelevanceReward
Good proposals. Number 1 seems strange for me, at least from the...
7.70.717.7

[ 6.3 WXDAI ]

@Keyrxng
Contributions Overview
ViewContributionCountReward
IssueComment16.3
Conversation Incentives
CommentFormattingRelevanceReward
The only other thing I can think of is just a more complex alter...
6.30.766.3

[ 1380.8 WXDAI ]

@rndquu
Contributions Overview
ViewContributionCountReward
IssueSpecification132.2
IssueTask11200
IssueComment3148.6
IssueComment30
Conversation Incentives
CommentFormattingRelevanceReward
User stories: - Partner should be able to set any ERC20 token a...
32.2
li:
  count: 9
  score: "9"
  words: 93
132.2
@0x4007 There are 4 options of how to implement permit fees: 1....
61.8
a:
  count: 4
  score: "4"
  words: 4
li:
  count: 12
  score: "12"
  words: 262
0.861.8
Thanks everyone. So we can't use custom permit2 contract with...
57.6
li:
  count: 8
  score: "8"
  words: 88
0.65557.6
> monthly recurring. For example, ideally, if we can enforce ...
29.2
li:
  count: 4
  score: "4"
  words: 30
code:
  count: 1
  score: "1"
  words: 1
0.8229.2
@0x4007 There are 4 options of how to implement permit fees: 1....
-
a:
  count: 4
  score: "0"
  words: 4
li:
  count: 12
  score: "0"
  words: 262
0.8-
Thanks everyone. So we can't use custom permit2 contract with...
-
li:
  count: 8
  score: "0"
  words: 88
0.655-
> monthly recurring. For example, ideally, if we can enforce ...
-
li:
  count: 4
  score: "0"
  words: 30
code:
  count: 1
  score: "0"
  words: 1
0.82-