This repository has been archived by the owner on May 22, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 23
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Moved parts of the code that are common and will be used for ETH bonding from KeepBonding to AbstractBonding contract. Only withdraw related functions remain in KeepBonding as they require different roles handling.
As TokenStaking contract has been refactored in keep-network/keep-core#1867 we can use Authorizations and StakeDelegatable contracts calls as these contracts define the functions we use in bonding. In this commit we use separately Authorizations and StakeDelegatable instead of TokenStaking to be more flexible and able to use common code from AbstractBonding for both KEEP token and ETH bonding.
nkuba
force-pushed
the
abstract-bonding
branch
from
August 11, 2020 09:19
15614dc
to
6655aca
Compare
pdyraga
reviewed
Aug 11, 2020
We added abstract functions for the calls we expect to make to staking contract. For KEEP token staking we expect calls to TokenStaking and for ETH staking calls to dedicated staking contract. We no longer need to pass two addresses: Authorizations and StakeDelegatable contract which seemed to be confusing.
Updated unit tests stubs to handle functions added to AbstractBonding contract that should implement calls to staking contract holding informations about delgation.
nkuba
force-pushed
the
abstract-bonding
branch
4 times, most recently
from
August 11, 2020 22:05
8655894
to
aa0503f
Compare
Solium complains about indentation problems but indentation is fine. Indentation is verified and fixed by prettier so we can ignore indentation cheks from oslium.
nkuba
force-pushed
the
abstract-bonding
branch
from
August 11, 2020 22:20
aa0503f
to
0befc5c
Compare
We will need to get to the functions from keep facotry and keeps.
pdyraga
approved these changes
Aug 12, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One non-blocking comment which can be addressed in the next PR.
revert("abstract function"); | ||
} | ||
|
||
function withdrawBondExposed(uint256 amount, address operator) public { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually do public*
in other tests, can we rename it to publicWithdrawBond
to keep it consistent?
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is preparation for ETH only staking. We extracted some common parts of
KeepBonding
contract that almost entirely can be used for ETH bonding and placed them inAbstractBonding
. The only difference, for now, iswithdraw
that in case of KEEP token staking has to support grantee roles check. This is an abstract function inAbstractBonding
implemented byKeepBonding
.Refs: #271