Skip to content
This repository has been archived by the owner on May 22, 2023. It is now read-only.

Abstract bonding contract #525

Merged
merged 10 commits into from
Aug 12, 2020
Merged

Abstract bonding contract #525

merged 10 commits into from
Aug 12, 2020

Conversation

nkuba
Copy link
Member

@nkuba nkuba commented Aug 11, 2020

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 in AbstractBonding. The only difference, for now, is withdraw that in case of KEEP token staking has to support grantee roles check. This is an abstract function in AbstractBonding implemented by KeepBonding.

Refs: #271

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.
@nkuba nkuba requested a review from pdyraga August 11, 2020 09:15
nkuba added 2 commits August 11, 2020 11:18
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 nkuba force-pushed the abstract-bonding branch from 15614dc to 6655aca Compare August 11, 2020 09:19
@nkuba nkuba self-assigned this Aug 11, 2020
@nkuba nkuba mentioned this pull request Aug 11, 2020
2 tasks
solidity/contracts/AbstractBonding.sol Outdated Show resolved Hide resolved
solidity/contracts/AbstractBonding.sol Outdated Show resolved Hide resolved
nkuba added 3 commits August 11, 2020 22:12
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 nkuba requested a review from pdyraga August 11, 2020 20:19
@nkuba nkuba force-pushed the abstract-bonding branch 4 times, most recently from 8655894 to aa0503f Compare August 11, 2020 22:05
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 nkuba force-pushed the abstract-bonding branch from aa0503f to 0befc5c Compare August 11, 2020 22:20
Copy link
Member

@pdyraga pdyraga left a 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 {
Copy link
Member

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?

@pdyraga pdyraga merged commit 1f4daf5 into master Aug 12, 2020
@pdyraga pdyraga deleted the abstract-bonding branch August 12, 2020 11:03
@pdyraga pdyraga added this to the v1.2.0 milestone Sep 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants