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

Dollar permits with fees #16

Closed
rndquu opened this issue Jun 12, 2024 · 13 comments · Fixed by ubiquity-os-marketplace/text-conversation-rewards#43
Closed

Dollar permits with fees #16

rndquu opened this issue Jun 12, 2024 · 13 comments · Fixed by ubiquity-os-marketplace/text-conversation-rewards#43

Comments

@rndquu
Copy link
Member

rndquu commented Jun 12, 2024

User stories:

  • Partner should be able to set any ERC20 token as a permit reward (we plan to support only USD-pegged coins initially)
  • Ubiquity owner should be able to set permit fees in some env variable. Notes:
    • Fees are transferred to Ubiquity treasury, if permits are regenerated then only the difference is sent
    • Use either direct transfer to ensure partner solvency either permits to the treasury
  • Ubiquity owner should be able to whitelist some ERC20 tokens so that they don't incur any fees (ex: Dollar token)

This issue touches the following repositories / docs:

@rndquu rndquu self-assigned this Jun 12, 2024
Copy link

ubiquibot bot commented Jun 12, 2024

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

@rndquu
Copy link
Member Author

rndquu commented Jun 12, 2024

@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
  1. 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
  1. 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 (?)
  1. 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
Copy link
Contributor

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
Copy link
Contributor

Keyrxng commented Jun 12, 2024

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
Copy link

EresDev commented Jun 12, 2024

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
Copy link

molecula451 commented Jun 13, 2024

  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
Copy link
Member

0x4007 commented Jun 13, 2024

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
Copy link
Member Author

rndquu commented Jun 13, 2024

Thanks everyone.

So we can't use custom permit2 contract with signed transfer details since it breaks (at least) ubiquity/pay.ubq.fi#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
Copy link
Member

0x4007 commented Jun 14, 2024

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.

  • Freemium model (meaning monthly recurring) is a very popular model for consumers (individuals/early startups) to try out the system.
  • Usage-based-fees seem popular for businesses.
  • Annual billing + dedicated support seemingly seems popular for enterprise.

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
Copy link
Member Author

rndquu commented Jun 26, 2024

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

Copy link

ubiquibot bot commented Jul 15, 2024

+ Evaluating results. Please wait...

Copy link

ubiquibot-dev bot commented Jul 15, 2024

[ 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
- -

Copy link

ubiquibot bot commented Jul 15, 2024

[ 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-

Keyrxng pushed a commit to ubq-testing/permit-generation that referenced this issue Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants