keep3r-network / keep3r.network Goto Github PK

Hi,

While reading through the Keep3r.sol code I found this require statement in the workReceipt function:

It seems like the amount that a job can reward a Keeper is capped at gasUsed.mul(uint(FASTGAS.latestAnswer())) .

Should it not be this instead:

require(
    amount >= KPRH.getQuoteLimit(_gasUsed.sub(gasleft())),
    "workReceipt: min limit"
);

Thus ensuring a minimum reward to cover gasCost for the Keeper, but not placing a maximum on what reward the job might want to pay out?

I noticed this when skimming so might also just be a misunderstanding from my zoomed in view.

keepers need to have an easy way to have invoices available to them for their work. https://request.network protocol can be leveraged to solve this in an elegant way.

Hi,

The StrategyCurveYVoterProxy contract for Strategy 0x07db4b9b3951094b9e278d336adf46a036295de7 (at least) is not working

When harvest is called, the first check is require(msg.sender == strategist || msg.sender == governance, "!authorized");

This does not allow a keeper to execute the harvest.

Hi, we recently have conducted a systematic study about Solidity event usage, evolution, and impact, and we are attempting to build a tool to improve the practice of Solidity event use based on our findings. We have tried our prototype tool on some of the most popular GitHub Solidity repositories, and for your repository, we find a potential optimization of gas consumption arisen from event use.

The point is that when we use emit operation to store the value of a certain variable, local memory type variable would be preferable to global storage type (state) variable if they hold the same value. The reason is that an extra SLOAD operation would be needed to access the variable if it is storage type, and the SLOAD operation costs 800 gas.

For your repository, we find that the following event use can be improved:

• HegicPoolKeep3r.sol
  function name: setMinRewards
  event name:  MinRewardsSet
  variable:    minETHRewards->_minETHRewards  minWBTCRewards->_minWBTCRewards

  function setMinRewards(uint256 _minETHRewards, uint256 _minWBTCRewards) public override onlyGovernor {
    minETHRewards = _minETHRewards;
    minWBTCRewards = _minWBTCRewards;
    emit MinRewardsSet(minETHRewards, minWBTCRewards);
  }

Do you find our results useful? Your reply and invaluable suggestions would be greatly appreciated, and are vital for improving our tool. Thanks a lot for your time!

In the README it is specified that bond can be 0, though keeper will never be able to activate himself according to this requirement

Keep3rV2Oracle is not to using FixedPoint library and it's using solc 0.8.x, this introduces a possible revert.

Last price cumulative's in uniswap / sushiswap pairs are accumulators, and they are designed to overflow.

When doing the calculations with these queried value, Keep3rV2Oracle does a priceLast * e10 / Q112 .

If the cumulative price is big enough * e10 will throw an overflow because of checked math in solidity 0.8.x, therefore reverting.

Very rough branch demonstrating that it will fail: link.

It does not represent an immediate risk but it depends a lot on pair's volume and tokens decimals. So, I'd strongly suggest to deprecate the oracle / fix this issue.

Hi!

Looking at the code it seems possible to bond any ERC20.
I'm wondering if it's possible to have a isMinKeeper check with a bond requirement of multiple coins
e.g. 1 kp3r + 1 gov token from X project.
So that a keeper job can require a keeper to have both kp3r and the gov token of the project the function is called on for incentivization alignment.
This mechanism could also be used as a registration / test / MACI for advisors and contributors.

Curious if this is supported / planned / possible?
Or some reason it's a bad idea?
Is an implementation the blocker? Front end / contract or both? Any guidance would be appreciated.

ping! sorry for the spam @andrecronje @Soleone @Soleone @matjaz @Macarse @xiaojay @fameal @RomanHiden @Macarse @mwalol @cleanunicorn

guardian == address(0x0). It is superfluous and useless
guardian will never address(0x0)

Hi, I would like to use contracts from this repo in our chain, however this repo does not specify license.
Would be possible to add license file into this repository?

As discussed in following StackExchange, the license file is necessary if creating of derived work should be legal:
https://opensource.stackexchange.com/a/1721

Also, SOL files compilation currently produce following warning when the license is missing in the SOL files headers:

Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: " to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.

Please, if you agree with considering this opensource, add the license to at least one of these places.
Thanks!

Price slippage too high

Hey guys. Is Keep3r network deployed on a testnet? How can I try it out?

This might lead to people thinking activate failed when they tried the first time and they might waste gas trying to activate again.

Unbonding is still pending after 14 days... can you please fix this?