Claim ERC20 tokens for all contributions at once #184

New Issue

2020-02-09T01:23:21Z

raucao commented

2020-02-09 01:23:21 +00:00

(Migrated from github.com)

@velasquez had a great idea for how we can greatly improve both efficiency and usability of withdrawing kredits for contributions. We sketched it out real quick and think this may work:

Instead of having to claim kredits for every single contribution, and minting them per contribution, we could introduce one new property to the contributor itself, which tracks how much you have withdrawn in the past. As the ERC20 token is fungible, it is irrelevant for which token you withdraw it in the first place. And this design means you can execute a single withdrawal for all of your available balance.

This is the idea with some pseudo code and instructions:

  struct Contributor {
    address account;
    bytes32 hashDigest;
    uint8 hashFunction;
    uint8 hashSize;
    bool exists;
    // TODO
    uint256 claimedBalance;
  }

  // TODO
  function withdraw() {
    // look up contributorId for msg.sender address
    // require msg.sender is contributor
    uint256 confirmedKredits = Contribution.totalKreditsEarnedByContributor(contributorId, confirmedOnly=true);
    uint256 claimableAmount = confirmedKredits - contributor.claimedBalance;
    // require claimableAmount > 0
    contributor.claimedBalance += claimableAmount;
    IToken(token).mintFor(msg.sender, claimableAmount);
  }

The claimable amount can also be put into a getter function, of course. This is just to illustrate the design.

@bumi @haythem96 What do you think?

@velasquez had a great idea for how we can greatly improve both efficiency and usability of withdrawing kredits for contributions. We sketched it out real quick and think this may work: Instead of having to claim kredits for every single contribution, and minting them per contribution, we could introduce one new property to the contributor itself, which tracks how much you have withdrawn in the past. As the ERC20 token is fungible, it is irrelevant for which token you withdraw it in the first place. And this design means you can execute a single withdrawal for all of your available balance. This is the idea with some pseudo code and instructions: ```solidity struct Contributor { address account; bytes32 hashDigest; uint8 hashFunction; uint8 hashSize; bool exists; // TODO uint256 claimedBalance; } // TODO function withdraw() { // look up contributorId for msg.sender address // require msg.sender is contributor uint256 confirmedKredits = Contribution.totalKreditsEarnedByContributor(contributorId, confirmedOnly=true); uint256 claimableAmount = confirmedKredits - contributor.claimedBalance; // require claimableAmount > 0 contributor.claimedBalance += claimableAmount; IToken(token).mintFor(msg.sender, claimableAmount); } ``` The claimable amount can also be put into a getter function, of course. This is just to illustrate the design. @bumi @haythem96 What do you think?

raucao commented

2020-02-15 17:47:21 +00:00

(Migrated from github.com)

@bumi @haythem96 Could you please have a look at this? I think it's a huge improvement over what we have, and it would be also be a shame to leave a new contributor hanging with zero feedback for too long.

bumi commented

2020-02-18 17:57:16 +00:00

(Migrated from github.com)

That's a great idea @velasquez I was trying to think about this and why we had that claimed attribute on the contribution.
In my experimental zapier script I am iterating over the contributions.
And I am always a bit scared iterating over an unknown amount of entries in solidity - do you see an issue there?

if we store a timestamp or the last claimed contribution then we could even know which contributions are unclaimed.

👍

That's a great idea @velasquez I was trying to think about this and why we had that `claimed` attribute on the contribution. In my [experimental zapier script](https://github.com/bumi/kredits-zapier/blob/master/creates/claim-contribution.js#L27) I am iterating over the contributions. And I am always a bit scared iterating over an unknown amount of entries in solidity - do you see an issue there? if we store a timestamp or the last claimed contribution then we could even know which contributions are unclaimed. :+1:

raucao commented

2020-02-18 17:59:49 +00:00

(Migrated from github.com)

if we store a timestamp or the last claimed contribution then we could even know which contributions are unclaimed.

Why would we have to know that in the first place? That's the beauty of this solution. We introduced a useless data point to begin with IMO.

> if we store a timestamp or the last claimed contribution then we could even know which contributions are unclaimed. Why would we have to know that in the first place? That's the beauty of this solution. We introduced a useless data point to begin with IMO.

raucao commented

2020-02-18 20:08:15 +00:00

(Migrated from github.com)

why we had that claimed attribute on the contribution

Was it because we planned that contributions can be created without an associated contributor account/address? The wording sounds a lot like that was the idea, but the current functionality is much more in line with a "withdraw/withdrawn" function and property than with "claim/claimed". (Hence the uselessness of knowing for which contribution you're withdrawing kredits.)

> why we had that `claimed` attribute on the contribution Was it because we planned that contributions can be created without an associated contributor account/address? The wording sounds a lot like that was the idea, but the current functionality is much more in line with a "withdraw/withdrawn" function and property than with "claim/claimed". (Hence the uselessness of knowing for which contribution you're withdrawing kredits.)

raucao added the feature label 2022-07-16 15:20:14 +00:00

raucao referenced a pull request that will close this issue

2022-08-25 09:46:59 +00:00

Remove claims, add withdrawals #225

raucao closed this issue

2022-08-30 08:53:55 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: kredits/contracts#184