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

feat: keeper reward for reservoir drip through token issuance #582

Merged
merged 21 commits into from
Sep 2, 2022

Conversation

pcarranzav
Copy link
Member

WIP as I still need to fix the tests.

This complements #571 adding a reward for whoever called drip(), to incentivise calling it and offset the gas costs. The reward is produced through additional token issuance that grows linearly with the number of blocks, after a minimum number of blocks since the last drip have passed.

Note the reward is credited on L1 if the l2RewardsFraction is set to 0. If l2RewardsFraction is nonzero, the reward will be credited in L2, and part of it will be given to the address that initiated the retryable ticket transaction, so that if a lazy or malicious keeper does not redeem the tx in L2, there is incentive for someone else to jump in and redeem it.

@pcarranzav pcarranzav force-pushed the pcv/drip-issuance-reward branch 2 times, most recently from 7d9d481 to 3054d93 Compare June 6, 2022 20:37
@pcarranzav pcarranzav force-pushed the pcv/drip-issuance-reward branch 2 times, most recently from e7759d3 to 74030db Compare June 6, 2022 21:40
@codecov
Copy link

codecov bot commented Jun 6, 2022

Codecov Report

Merging #582 (80cc2f2) into pcv/arb-bridge (500e86b) will increase coverage by 0.18%.
The diff coverage is 100.00%.

❗ Current head 80cc2f2 differs from pull request most recent head b99d31f. Consider uploading reports for the commit b99d31f to get more accurate results

@@                Coverage Diff                 @@
##           pcv/arb-bridge     #582      +/-   ##
==================================================
+ Coverage           91.96%   92.14%   +0.18%     
==================================================
  Files                  44       44              
  Lines                2092     2140      +48     
  Branches              361      373      +12     
==================================================
+ Hits                 1924     1972      +48     
  Misses                168      168              
Flag Coverage Δ
unittests 92.14% <100.00%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
contracts/reservoir/Reservoir.sol 100.00% <ø> (ø)
contracts/l2/reservoir/L2Reservoir.sol 100.00% <100.00%> (ø)
contracts/reservoir/L1Reservoir.sol 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@pcarranzav
Copy link
Member Author

(Fixed the tests, but I still need to add a test for distributing the keeper reward in L2)

@pcarranzav
Copy link
Member Author

This won't work as it is, because tx.origin will be set by ArbOS to the origin from L1. We're discussing alternatives with the Offchain Labs folks, but we might have to only give rewards to the L1 keeper.

@pcarranzav
Copy link
Member Author

Marking this as ready for review; it matches the latest iteration of what's described in GIP-0034 (The OP Forum post has an outdated version, latest on HackMD)

@pcarranzav pcarranzav marked this pull request as ready for review June 29, 2022 16:12
@pcarranzav pcarranzav changed the base branch from pcv/arb-full-deployment to pcv/571-n23-whitelist-check July 14, 2022 15:51
@pcarranzav
Copy link
Member Author

Rebased on top of #624 to incorporate all the audit fixes. This caused lot of merge conflicts that needed to be solved...

@pcarranzav pcarranzav force-pushed the pcv/drip-issuance-reward branch 3 times, most recently from b657902 to 29b8373 Compare July 15, 2022 12:22
@pcarranzav pcarranzav changed the base branch from pcv/571-n23-whitelist-check to pcv/571-safemath-consistency July 15, 2022 12:22
@socket-security
Copy link

socket-security bot commented Jul 28, 2022

Socket Security Report

👍 No new dependency issues detected in pull request

Socket.dev scan summary
Issue Status
Did you mean? ✅ no new possible package typos
Install scripts ✅ no new install scripts
Telemetry ✅ no new telemetry
Troll package ✅ no new troll packages
Malware ✅ no new malware
Native code ✅ no new native modules

Powered by socket.dev

@pcarranzav pcarranzav changed the base branch from pcv/571-safemath-consistency to pcv/arb-bridge July 28, 2022 13:40
@pcarranzav pcarranzav mentioned this pull request Jul 28, 2022
@abarmat abarmat added enhancement New feature or request protocol change labels Aug 5, 2022
Copy link
Contributor

@abarmat abarmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@pcarranzav pcarranzav merged commit b99d31f into pcv/arb-bridge Sep 2, 2022
@pcarranzav pcarranzav deleted the pcv/drip-issuance-reward branch September 2, 2022 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request protocol change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants