ECDSAKeep Rewards #269

NicholasDotSol · 2020-03-02T22:11:27Z

Refs: #446
ECDSAKeep Rewards to distribute KEEP tokens to keep owners based on a predetermined curve separated by intervals.

Draft until:

fully tested
documented
cleaned up
optimized

Used by ECDSAKeepRewards to get the owner of the keep, and the time it was opened.

Track all created keeps and timestamps. These are used for rewards and will be utilized furhter in the future.

Expose functions to get basic info on keeps and timestamps from the internal data structures.

stubOpenKeep is an easy way to open a keep with no extra requirements. It's beneficial when mny keeps are needed and the initializatoin logic is not important.

Draft of ECDSAKeepRewards. findInterval not implemented.

Test the binary search custom functinality.

Arbitrary timestamps permit the creation of data to deliberately exercise edge cases.

Importing via `@openzeppelin` makes Go bindings code generation fails as the path for `@openzeppelin` is not configured in the Makefile. I could add `@openzeppelin` path to the Makefile but since all other contracts use `openzeppelin-solidity` import, I altered the single `@openzeppelin` import to `openzeppeling-solidity for consistency.

KeepTerminated event is handled the same way as KeepClosed event exept that it logs warning on the console.

pdyraga · 2020-03-14T16:36:24Z

Tested off-chain client against the changes, everything works as expected.

Test log

$ truffle exec integration/smoke_test.js --network local
Using network 'local'.

selecting a keep factory...
validate registered member candidates...
operator [0x65EA55c1f10491038425725dC00dFFEAb2A1e28A] is registered for application [0x146748a2b46B99ee1470b587bc9812Ea59b79597]
operator [0x524f2E0176350d950fA630D9A5a59A0a190DAf48] is registered for application [0x146748a2b46B99ee1470b587bc9812Ea59b79597]
operator [0x3365D0Ed0e526D3B1d8b417fc0fdE5b1cEF2f416] is registered for application [0x146748a2b46B99ee1470b587bc9812Ea59b79597]
opening a new keep for owner [0x7020A5556Ba1ce5f92c81063a13d33512cf1305c]...
open new keep fee: [11516250000000000]
new keep opened with address: [0x5E9033CE4f88cFDaa734ec7F0905e9845771D884] and members: [0x65EA55c1f10491038425725dC00dFFEAb2A1e28A,0x524f2E0176350d950fA630D9A5a59A0a190DAf48,0x3365D0Ed0e526D3B1d8b417fc0fdE5b1cEF2f416]
get public key...
public key generated for keep: [0x35f5c98c67a84590ff5fb6d726c3a88323ae31145571ab1322c523b72495ed9ccac925819b8941e925a2aed35f4ae2a1f0f13d6bad396c5c7919258ffca9eadc]
request signature...
received valid signature:
r: [0x8afe3457efca609b203092667caf77eab48725260862c88b2c230382e6044cb9]
s: [0x059711b2c8ba20265b8c53155050fefbc67d085b1e39330246ada1f9d4576bfd]
recoveryID: [1]

wait for new relay entry generation...
get current group selection seed...
group selection seed was successfully updated by the random beacon

$ cat close_keep.js 
const BondedECDSAKeep = artifacts.require('BondedECDSAKeep.sol');

module.exports = async function() {

    const keepAddress = "0x5E9033CE4f88cFDaa734ec7F0905e9845771D884"
    const keepOwner = "0x7020A5556Ba1ce5f92c81063a13d33512cf1305c"

    try {
        keep = await BondedECDSAKeep.at(keepAddress)
        await keep.closeKeep({from: keepOwner})

        console.log('keep closed')
    } catch(error) {
        console.log(error)
    }
};

$ truffle exec close_keep.js --network local
Using network 'local'.

keep closed

On the client:

17:22:25.344  INFO keep-tecds: keep [0x5E9033CE4f88cFDaa734ec7F0905e9845771D884] closed client.go:310
17:22:25.344  INFO keep-tecds: unsubscribing from events on keep closed client.go:329

$ truffle exec integration/smoke_test.js --network local
Using network 'local'.

Browserslist: caniuse-lite is outdated. Please run next command `npm update`
selecting a keep factory...
validate registered member candidates...
operator [0x65EA55c1f10491038425725dC00dFFEAb2A1e28A] is registered for application [0x146748a2b46B99ee1470b587bc9812Ea59b79597]
operator [0x524f2E0176350d950fA630D9A5a59A0a190DAf48] is registered for application [0x146748a2b46B99ee1470b587bc9812Ea59b79597]
operator [0x3365D0Ed0e526D3B1d8b417fc0fdE5b1cEF2f416] is registered for application [0x146748a2b46B99ee1470b587bc9812Ea59b79597]
opening a new keep for owner [0x7020A5556Ba1ce5f92c81063a13d33512cf1305c]...
open new keep fee: [11516250000000000]
new keep opened with address: [0x9E8E3487dCCd6a50045792fAfe8Ac71600B649a9] and members: [0x3365D0Ed0e526D3B1d8b417fc0fdE5b1cEF2f416,0x65EA55c1f10491038425725dC00dFFEAb2A1e28A,0x524f2E0176350d950fA630D9A5a59A0a190DAf48]
get public key...
public key generated for keep: [0x9d8e332d3ad0eca30cfd1f9304d34d5a40821688e4bcaab333fcd582cbca9de919b30c725541d71d4913f021ed7db3524a25496f4e0ea9af767f0596a51e6999]
request signature...
received valid signature:
r: [0x0a7dfb771d112d99d4ce458b130795e52440aa35bcdecaa4049f39691be35b4a]
s: [0x7b60414653f1251c4ab2dac275fd2bec2a74f421504dcb3f13c61019976a9492]
recoveryID: [1]

wait for new relay entry generation...
get current group selection seed...
group selection seed was successfully updated by the random beacon

$ cat seize_bonds.js 
const BondedECDSAKeep = artifacts.require('BondedECDSAKeep.sol');

module.exports = async function() {

    const keepAddress = "0x9E8E3487dCCd6a50045792fAfe8Ac71600B649a9"
    const keepOwner = "0x7020A5556Ba1ce5f92c81063a13d33512cf1305c"

    try {
        keep = await BondedECDSAKeep.at(keepAddress)
        await keep.seizeSignerBonds({from: keepOwner})

        console.log('bonds seized')
    } catch(error) {
        console.log(error)
    }
};

$ truffle exec seize_bonds.js --network local 
Using network 'local'.

bonds seized

On the client:

17:25:44.352 WARNI keep-tecds: keep [0x9E8E3487dCCd6a50045792fAfe8Ac71600B649a9] terminated client.go:346
17:25:44.353  INFO keep-tecds: unsubscribing from events on keep terminated client.go:365

I still need to go through the Rewards contract.

pdyraga

We need to add more docs and explain in the PR description how this mechanism works.

pdyraga · 2020-03-14T16:40:11Z

solidity/contracts/ECDSAKeepRewards.sol

+import "openzeppelin-solidity/contracts/token/ERC20/IERC20.sol";
+import "openzeppelin-solidity/contracts/math/SafeMath.sol";
+
+contract ECDSAKeepRewards {


It's worth adding some documentation to this contract.

pdyraga · 2020-03-14T17:03:40Z

solidity/contracts/ECDSAKeepRewards.sol

+        _processKeep(true, keepAddress);
+    }
+
+    function reportTermination(address keepAddress)


We should add documentation to this function.

Do we expect it's called by parties interested in eventually claiming something from unallocated rewards?

Yes; the rewards allocated for that keep will be returned to the pool of unallocated rewards.

pdyraga · 2020-03-14T17:16:13Z

solidity/contracts/ECDSAKeepRewards.sol

+    // Rewards that haven't been allocated to finished intervals
+    uint256 unallocatedRewards;
+
+    // Length of one interval.


... in seconds ?

pdyraga · 2020-03-14T17:18:49Z

solidity/contracts/ECDSAKeepRewards.sol

+    // Total number of intervals. (Implicit in intervalWeights)
+    // uint256 termCount = intervalWeights.length;
+
+    // mapping of keeps to booleans. True if the keep has been used to calim a reward.


s/calim/claim

pdyraga · 2020-03-14T17:22:29Z

solidity/test/RewardsTest.js

+            let keepAddress = await keepFactory.getKeepAtIndex(0)
+            let eligible = await rewards.eligibleForReward(keepAddress)
+            expect(eligible).to.equal(false)
+        })


We should cover the following cases as well:

keep not recognized by the factory,

reward already claimed.

pdyraga · 2020-03-14T18:47:56Z

solidity/contracts/ECDSAKeepRewards.sol

+       }
+   }
+
+   function _getIntervalWeight(uint256 interval) internal view returns (uint256) {


What is interval weight and why is it 100% for the interval above their count?

Interval weight is the percentage of remaining unallocated rewards that are to be allocated for that interval. Once the defined schedule is over, any leftover rewards (e.g. from terminated keeps or division remainders) are allocated to the earliest available interval.

pdyraga · 2020-03-14T18:49:33Z

solidity/test/RewardsTest.js

+        50, // 40:40
+        25, // 10:30
+        50, // 15:15
+    ]


I do understand allocated : remaining relation but I have no idea how does it relate to interval weights. Shouldn't weights add up to 100%?

No; weights are how much of the remaining unallocated rewards are allocated for that interval. After the initially allocated schedule is over, the allocation jumps to 100% meaning that all remaining unallocated rewards are allocated for each closing interval.

pdyraga · 2020-03-14T18:56:13Z

solidity/test/RewardsTest.js

+    let bondedSortitionPoolFactory
+    let keepBonding
+    let randomBeacon
+    let signerPool


pdyraga · 2020-03-14T18:56:20Z

solidity/test/RewardsTest.js

+    const aliceBeneficiary = accounts[2]
+    const bobBeneficiary = accounts[3]
+
+    let factory


pdyraga · 2020-03-14T19:06:30Z

solidity/contracts/ECDSAKeepRewards.sol

+
+    /// @notice Sends the reward for a keep to the keep members.
+    /// @param keepAddress ECDSA keep factory address.
+    function receiveReward(address keepAddress)


Who will call this function? From what I understand, it does not make sense to call this function before all previous keeps are closed or terminated. If there is a lot of active keeps, we never know if some of them will be terminated and rewards potentially increased.

Because of how rewards are allocated for the intervals, the outcome for keeps in a closed interval won't change even if the unallocated amount increases. That means that keeps can call receiveReward as soon as the keep is closed and the interval is over, without needing to wait for other keeps.

pdyraga · 2020-03-28T17:46:45Z

I extracted parts about the client, keep and factory contracts to a separate PR: #333. Reviewing this one should be easier once #333 gets merged.

pdyraga · 2020-10-01T08:09:42Z

Closing in favor of #513

NicholasDotSol added 7 commits March 2, 2020 15:45

ECDSAKeep Getters

994158b

Used by ECDSAKeepRewards to get the owner of the keep, and the time it was opened.

Track keeps and timestamps

44c3ac7

Track all created keeps and timestamps. These are used for rewards and will be utilized furhter in the future.

Add keep info getters

c4dfa6b

Expose functions to get basic info on keeps and timestamps from the internal data structures.

Simple openKeep stub

ca96c9f

stubOpenKeep is an easy way to open a keep with no extra requirements. It's beneficial when mny keeps are needed and the initializatoin logic is not important.

ECDSAKeepRewards Draft

dbdf3cc

Draft of ECDSAKeepRewards. findInterval not implemented.

Test ECDSAKeepRewards.find

7e3e974

Test the binary search custom functinality.

Merge branch 'master' into rewards

ee28076

NicholasDotSol requested a review from eth-r March 2, 2020 22:11

eth-r and others added 22 commits March 3, 2020 16:03

Record keep address and creation time in factory

2f438dd

Use arbitrary timestamps for rewards keep factory

9efe93c

Arbitrary timestamps permit the creation of data to deliberately exercise edge cases.

Make tests pass with an optimized search algorithm

e549326

Add test case for duplicated timestamps

b2906c7

Document Rewards functions

0a4c2ac

Merge branch 'rewards' into rewards-promethea

dba113f

Skip keep factory tests for now

07150e9

Set arbitrary timestamps in keeps too, not just factories

072fa1d

Remove unused code

70b2f2e

Add function to calculate intervals from timestamps

e938d90

Add test for intervalOf revert

49a369f

Add functions for handling interval endpoints

2fda500

Add test for getEndpoint reverting for ongoing intervals

638e4d6

Add more tests for getInterval

a6bdf27

Fix endpoint test typo and add function to get previous endpoint

74ffb9e

Add state variables for tracking reward payouts

38ed6c6

Add a Status enum and state variable for ECDSA keeps

69d8ab2

Replace isActive flag with status in ECDSA keep

e8641a6

Add test stub functions to close or terminate the keep

0cd76ea

Require that keep was closed happily to be eligible for rewards

cb5ffd5

Add functions and tests for interval weights and interval count

7e6c07f

Add counting of keeps per interval and basic reward allocation

e05e954

pdyraga added 4 commits March 14, 2020 15:48

Emit KeepTerminated event when terminating a keep

009bc62

Test making sure factory produces active keeps

9449087

Handle KeepTerminated event in the client

6931085

KeepTerminated event is handled the same way as KeepClosed event exept that it logs warning on the console.

pdyraga reviewed Mar 14, 2020

View reviewed changes

eth-r added 13 commits March 16, 2020 17:10

Add a way to fund rewards with approveAndCall

c913092

Fix variable shadowing warnings

1ef630f

Improve documentation of ECDSAKeepRewards

29df9ae

Remove a magic number in reward tests and reorder them

8c5b84b

Add more tests for receiveReward

8f6d901

Add tests for reportTermination

20616e6

Rename end* to start* in startOf tests

16d7bdc

Remove unused variables

a425571

Merge branch 'master' into rewards

8f62e2f

Add documentation of the reward contract and its usage

9041e12

Add tests for not processing a keep before the interval is over

ff71a21

Remove .only from ECDSAKeepRewards test

416a39f

Remove unused code and improve documentation

b15d7db

eth-r requested a review from pdyraga March 17, 2020 15:51

pdyraga mentioned this pull request Mar 28, 2020

Add a concept of terminated keep, track keep creation timestamps in the factory #333

Merged

pdyraga added the ⛓chain label Apr 10, 2020

Shadowfiend changed the base branch from master to ci-fetch-external May 28, 2020 19:40

Shadowfiend changed the base branch from ci-fetch-external to master May 28, 2020 19:40

eth-r added 2 commits July 28, 2020 15:48

Initial conversion of ECDSA rewards to use abstract rewards

8048acf

Update Registry to KeepRegistry in imports

81c6142

pdyraga closed this Oct 1, 2020

pdyraga deleted the rewards branch October 1, 2020 08:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ECDSAKeep Rewards #269

ECDSAKeep Rewards #269

NicholasDotSol commented Mar 2, 2020 •

edited by pdyraga

Loading

pdyraga commented Mar 14, 2020

pdyraga left a comment

pdyraga Mar 14, 2020

pdyraga Mar 14, 2020

eth-r Mar 16, 2020

pdyraga Mar 14, 2020

pdyraga Mar 14, 2020

pdyraga Mar 14, 2020

pdyraga Mar 14, 2020

eth-r Mar 16, 2020

pdyraga Mar 14, 2020

eth-r Mar 16, 2020

pdyraga Mar 14, 2020

pdyraga Mar 14, 2020

pdyraga Mar 14, 2020

eth-r Mar 16, 2020

pdyraga commented Mar 28, 2020

pdyraga commented Oct 1, 2020

ECDSAKeep Rewards #269

ECDSAKeep Rewards #269

Conversation

NicholasDotSol commented Mar 2, 2020 • edited by pdyraga Loading

pdyraga commented Mar 14, 2020

pdyraga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdyraga commented Mar 28, 2020

pdyraga commented Oct 1, 2020

NicholasDotSol commented Mar 2, 2020 •

edited by pdyraga

Loading