-
Notifications
You must be signed in to change notification settings - Fork 4
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
[WIP]BEP153: Native Staking Implementation #7
[WIP]BEP153: Native Staking Implementation #7
Conversation
contracts/Staking.sol
Outdated
struct TransferInUndelegatedSynPackage { | ||
uint256 amount; | ||
address recipient; | ||
address refundAddr; |
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.
So be here, refundAddr
is redundant.
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.
Fixed
contracts/TokenHub.sol
Outdated
@@ -79,6 +79,8 @@ contract TokenHub is ITokenHub, System, IParamSubscriber, IApplication, ISystemR | |||
|
|||
uint256 public relayFee; | |||
|
|||
uint256 public totalStakingBNB; |
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.
The newly defined variable should be at the end of the variables declarations, so I think you should move this behind line86
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.
Fixed.
contracts/Staking.sol
Outdated
} | ||
|
||
function undelegate(address validator, uint256 amount) override external payable tenDecimalPrecision(amount) initParams { | ||
require(pendingUndelegated[msg.sender][validator] == 0, "pending undelegation exist"); |
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.
Is this necessary?
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.
Yes. BC will not accept two undeledation at one time. So we'd better check that on BSC in advance
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.
ok
contracts/Staking.sol
Outdated
require(validatorSrc!=validatorDst, "invalid redelegation"); | ||
amount = amount != 0 ? amount : delegatedOfValidator[msg.sender][validatorSrc]; | ||
if (amount < minDelegationChange) { | ||
require(amount == delegatedOfValidator[msg.sender][validatorSrc], |
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.
I would better like that when the delegatedOfValidator[msg.sender][validatorSrc] is little than minDelegationChange, the reldelegate action should be forbidden.
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.
If so, then when the balance is less than minDelegationChange, the delegator must undelegate first to get his balance back
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.
I think so.
contracts/Staking.sol
Outdated
|
||
function undelegate(address validator, uint256 amount) override external payable tenDecimalPrecision(amount) initParams { | ||
require(pendingUndelegated[msg.sender][validator] == 0, "pending undelegation exist"); | ||
amount = amount != 0 ? amount : delegatedOfValidator[msg.sender][validator]; |
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.
Because the msg.sender can get the exact value, the amount should be specified precisely. I think there is no need special case for 0 amount, just forbid it.
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.
Ok
contracts/Staking.sol
Outdated
|
||
function redelegate(address validatorSrc, address validatorDst, uint256 amount) override external payable tenDecimalPrecision(amount) initParams { | ||
require(validatorSrc!=validatorDst, "invalid redelegation"); | ||
amount = amount != 0 ? amount : delegatedOfValidator[msg.sender][validatorSrc]; |
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.
The same here.
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.
Ok
|
||
amount = distributedReward[msg.sender]; | ||
payable(msg.sender).transfer(amount); | ||
distributedReward[msg.sender] = 0; |
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.
Better first set distributedReward[msg.sender] = 0; then payable(msg.sender).transfer(amount);
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.
Done
|
||
amount = undelegated[msg.sender]; | ||
payable(msg.sender).transfer(amount); | ||
undelegated[msg.sender] = 0; |
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.
The same here.
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.
Done
contracts/Staking.sol
Outdated
|
||
uint256 constant public TEN_DECIMALS = 1e10; | ||
|
||
uint256 public constant INIT_ORACLE_RELAYER_FEE = 2e16; //TODO |
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.
Delete TODO
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.
It's to be decided
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.
2e16 is fine.
contracts/Staking.sol
Outdated
uint256 constant public TEN_DECIMALS = 1e10; | ||
|
||
uint256 public constant INIT_ORACLE_RELAYER_FEE = 2e16; //TODO | ||
uint256 public constant INIT_MIN_DELEGATION_CHANGE = 1e20; |
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.
100 * 1e18 is more readable?
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.
Ok
contracts/Staking.sol
Outdated
require(success, "rlp decode ack package failed"); | ||
|
||
delegated[pack.delegator] = delegated[pack.delegator].add(pack.amount); | ||
pendingUndelegated[pack.delegator][pack.validator] = 0; |
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.
Better protect, require( pendingUndelegated[pack.delegator][pack.validator] == pack.amount, "")
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.
Ok
contracts/Staking.sol
Outdated
@@ -18,19 +18,24 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication { | |||
using RLPEncode for *; | |||
using RLPDecode for *; | |||
|
|||
// Cross-Chain Stake Event type | |||
// Package type | |||
uint8 constant public SYN_PACKAGE = 0x00; |
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 can delete this.
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.
LGTM
contracts/Staking.sol
Outdated
} else if (eventType == EVENT_DISTRIBUTE_UNDELEGATED) { | ||
(resCode, ackPackage) = _handleDistributeUndelegatedSynPackage(iter); | ||
} else { | ||
require(false, "unknown event type"); |
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.
it's better to use revert("unknown event type");
contracts/Staking.sol
Outdated
if (paramBytes.hasNext()) { | ||
iter = paramBytes.next().toBytes().toRLPItem().iterator(); | ||
} else { | ||
require(false, "empty fail ack package"); |
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.
so be here
contracts/Staking.sol
Outdated
if (paramBytes.hasNext()) { | ||
iter = paramBytes.next().toBytes().toRLPItem().iterator(); | ||
} else { | ||
require(false, "empty fail ack package"); |
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.
so be here
contracts/Staking.sol
Outdated
if (paramBytes.hasNext()) { | ||
iter = paramBytes.next().toBytes().toRLPItem().iterator(); | ||
} else { | ||
require(false, "empty fail ack package"); |
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.
so be here
contracts/Staking.sol
Outdated
require(newMinDelegation > 0, "the minDelegation must be greater than 0"); | ||
minDelegation = newMinDelegation; | ||
} else { | ||
require(false, "unknown param"); |
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.
so be here
contracts/Staking.sol
Outdated
uint32 public constant ERROR_WITHDRAW_BNB = 101; | ||
|
||
uint256 public constant TEN_DECIMALS = 1e10; | ||
uint256 public constant LOCK_TIME = 691200; // 8*24*3600 |
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.
it may be better to use 8 days
to replace 691200
contracts/Staking.sol
Outdated
|
||
uint256 convertedAmount = amount.div(TEN_DECIMALS); // native bnb decimals is 8 on BBC, while the native bnb decimals on BSC is 18 | ||
uint256 _relayerFee = (msg.value).sub(amount); | ||
uint256 oracleRelayerFee = _relayerFee-bSCRelayerFee; |
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.
better to use SafeMath
contracts/Staking.sol
Outdated
|
||
uint256 convertedAmount = amount.div(TEN_DECIMALS); // native bnb decimals is 8 on BBC, while the native bnb decimals on BSC is 18 | ||
uint256 _relayerFee = msg.value; | ||
uint256 oracleRelayerFee = _relayerFee-bSCRelayerFee; |
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.
better to use SafeMath
contracts/Staking.sol
Outdated
delegateInFly[msg.sender] += 1; | ||
|
||
ICrossChain(CROSS_CHAIN_CONTRACT_ADDR).sendSynPackage(CROSS_STAKE_CHANNELID, msgBytes, oracleRelayerFee.div(TEN_DECIMALS)); | ||
payable(TOKEN_HUB_ADDR).transfer(amount+oracleRelayerFee); |
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.
better to use SafeMath
contracts/Staking.sol
Outdated
|
||
uint256 convertedAmount = amount.div(TEN_DECIMALS);// native bnb decimals is 8 on BBC, while the native bnb decimals on BSC is 18 | ||
uint256 _relayerFee = msg.value; | ||
uint256 oracleRelayerFee = _relayerFee-bSCRelayerFee; |
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.
better to use SafeMath
"0x88cb4D8F77742c24d647BEf8049D3f3C56067cDD": { | ||
"balance": "0x100000000000000000000" | ||
}, | ||
"0x42D596440775C90db8d9187b47650986E1063493": { |
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.
I am wondering if of most the BSC validators will use this genesis-template.json
to generate genesis Genesis
. If yes, does that mean wallet 0x42D596440775C90db8d9187b47650986E1063493
will have 1000 BNB in their balance?
return false; | ||
} | ||
delete packageQueue[leftIndex]; | ||
++leftIndex; |
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.
_checkPackHash
requires strict timing[one after another] of these events:
- BSC sending "EVENT_DELEGATE 1" to BC
- BC sending
AckPackage/FailAckPackage 1
to BSC - BSC sending "EVENT_DELEGATE 2" to BC
- BC sending
AckPackage/FailAckPackage 2
to BSC - BSC sending "EVENT_DELEGATE 3" to BC
- BC sending
AckPackage/FailAckPackage 3
to BSC
Not sure if there is a scenario: events can be processed in a different order. For example:
- BSC sending "EVENT_DELEGATE 1" to BC
- BSC sending "EVENT_DELEGATE 2" to BC
- BC sending
AckPackage/FailAckPackage 2
to BSC <=AckPackage 2
received beforeAckPackage 1
. - BC sending
AckPackage/FailAckPackage 1
to BSC - BSC sending "EVENT_DELEGATE 3" to BC
- BC sending
AckPackage/FailAckPackage 3
to BSC
If this happens, _checkPackHash
will always return false
and handleAckPackage
will revert forever.
1. Summary
This BEP introduces a native staking protocol onto BNB Smart Chain. With this BEP, individual or institution delegators can stake BNB to specified validators and get staking rewards on the BSC side directly.
2. Abstract
This BEP introduces a new staking system contract on the BSC side, all staking-related operations on the BSC side should be initiated through this contract, and then applied across-chain to BNB Beacon Chain through the native cross-chain communication mechanism. The cross-chain staking APP on the BNB Beacon Chain side reuses the previous staking mechanism to handle these staking-related operations.
The goals of this BEP is:
3. Status
This BEP is a draft.
4.Motivation
Before this BEP, the BNB holders can only stake their assets on the BNB Beacon Chain side, which means that if their assets are on the BNB Smart Chain side, they have to transfer their assets across-chain to the BNB Beacon Chain first, which is not user-friendly enough.
With this BEP, the BNB holders can stake on the BSC side directly, and dApps can launch their staking service based on the protocol introduced by this BEP, which can diversify the BSC ecosystem.
5. Specification
5.1 Overview
There are mainly four components:
5.2 Staking System Contract
This is a new system contract deployed on BSC. It fulfills all the basic functionalities of staking service including delegation, undelegation, claiming reward and redelegation. It receives requests, encapsulates data and sends cross-chain packages. Specifically, the cross-chain package is normalized as a Cross-Chain Stake Event as below.
5.2.1 Cross-Chain Stake Event
The Cross-chain stake event will be emitted once a cross-chain stake transaction happens on BSC. It is mainly composed of channel ID, sequence ID and payload data.
The payload data is a sequence of RLP encoded bytes of {eventType, params}.
eventType: uint8, will determine the corresponding handler that handles staking requests, and also determine how to decode the data. It includes the following types.
Among them,
distribute reward
anddistribute undelegated
events will be emitted initially on BC.5.2.2 Data Structs
To keep track of delegators staking funds, we adopt multiple mappings to record data.
delegated
records every delegator’s total staked amount. When a delegator submits undelegation, his/her delegated amount will decrease and after undelegation finishes, the undelegated increase the same number.delegatedOfValidator
records every delegator’s staked amount to a specific validator.distributedReward
records every delegator’s reward that is already distributed to BSC.pendingUndelegateTime
andpendingRedelegateTime
records the estimated finish time of undelegation/redelegation. A new operation must wait until the previous operation is closed(until the time is up or receive an ack/failack package). Please be noted that this record will not always be accurate. The previous operation may end early.undelegated
records every delegator’s unlocked undelegate funds that is already distributed to BSC.5.2.3 Staking System Contract APIs
delegate(address validator, uint256 amount) payable
This method will handle the cross-chain delegating requests. Here, the
validator
represents the BC operator address of the validator to be delegated to. Cross-chain relayer fee should be included in the msg.value. The following graph represents the workflow of delegation on BSC:undelegate(address validator, uint256 amount) payable
This method will handle the cross-chain undelegating requests. The staked BNBs on BC will be unlocked after 7 days and transferred to the staking contract on BSC automatically. Delegators could claim undelegated BNBs by
claimUndelegated
(see below). Thevalidator
represents the BC operator address of the validator delegated to. Cross-chain relayer fee should be included in the msg.value.redelegate(address validatorSrc, address validatorDst, uint256 amount) payable
Undelegate amount BNBs from
validatorSrc
and delegate tovalidatorDst
.claimReward() returns(uint256 amount)
Delegators can claim their distributed reward by this method. The returned amount is the number of funds that
msg.sender
received.claimUndelegated() returns(uint256 amount)
Delegators can claim their undelegated funds by this method. The returned amount is the number of funds that
msg.sender
received.getDelegated(address delegator, address validator) external view returns(uint256 amount)
This function allows the delegator to query how much he/she has delegated to the specific validator.
getTotalDelegated(address delegator) external view returns(uint256 amount)
This function allows the delegator to query how much he/she has delegated in all.
getDistributedReward(address delegator) external view returns(uint256 amount)
This function allows the delegator to view the amount of the distributed rewards.
getPendingRedelegateTime(address delegator, address valSrc, address valDst) external view returns(uint256 amount)
This function allows the delegator to view the estimated finish time of redelegation from
valSrc
tovalDst
.getPendingUndelegateTime(address delegator, address validator) external view returns(uint256 amount)
This function allows the delegator to view the estimated finish time of undelegation from
validator
.getUndelegated(address delegator) external view returns(uint256 amount)
This function allows the delegator to view the amount of the undelegated funds.
5.3 Cross-Chain Staking APP
This is a new cross-chain app on BC side. It will be called by the BC’s bridge once it receives cross-chain staking packages. The main function of this app is to parse the data and do the staking related tasks.
5.3.1 Staking
The native staking mechanism on the BC will stay almost the same as before. The only change is that the delegation will be tagged as native or not native(cross-chain). The Cross-Chain Staking app will decode the cross-chain package depending on the event type code and call corresponding functions.
5.3.2 Contract Address on BC(CAoB)
The delegator’s corresponding address on the BC will be the XOR result between a map hash of a specific constant string and its BSC address. So no one could control it except for the cross-chain app.
5.3.3 Distribute Reward
The delegation reward will be released at each breath block to the reward CAoB, once the balance of the reward CAoB is larger than a threshold. The reward will be cross-chain transferred to the staking system contract on the BSC side automatically.
5.3.4 Distribute Undelegated
When undelegation finishes, the undelegated funds will be released to the delegate CAoB address, and the funds of the delegate CAoB will be cross-chain transferred to the staking system contract on the BSC side automatically in the next breath block.
5.4 Fees
The fees charged for the relevant transactions are based on the following principles:
5.5 Error Handle
This BEP will reuse the existing cross-chain infrastructure as much as possible. This includes and is not limited to the separation of application and communication protocol layers, uniform error handling principle and cross-chain application coding principle. As the new system contract BSC and new app on BC are all parts of the application layer, the error handling of the communication layer will remain the same as before.
5.5.1 Normal Error
Normal errors means anticipated errors. For transactions that cross-chain communication is involved, all normal errors will be relayed back. So no errors will be missed. For this type of errors, we recommend that developers should check the returned reason for failure, modify the parameters and retry the transaction. All anticipated errors and success scenarios are listed below.
Therefore, if any errors happen on BSC, the transaction will be reverted and the msg sender will know it at once. But if an error happens on BC, the msg sender will not get feedback immediately. So we recommend dApp developers run some robots to monitor related events to get informed.
5.5.2 Crash in Application Layer
The communication layer will catch the crash error of the application, record it in store or emit an event for debugging. The crash error will use the FAIL_ACK type of package if needed, and the payload of FAIL_ACK is the same as the payload of the SYNC package.
For contracts in BSC, the crash of Application will revert state change automatically, but for Application on BC we need to build another sandbox for it, only commit store after it did not crash.
When receiving the FAIL_ACK package, the communication layer will invoke a different method of Application, like handleFailAckPackage or ExecuteFailAckClaim. We will ensure the safety of users’ assets(see 5.5.4).
5.5.3 Error in Communication Layer
Communication layer only plays around with these inputs: Type, Sequence, ChannelId, RelayerFee. The simplicity of these inputs helps us build a robust system.
However, once an error happens, the communication of a channel may be blocked. We have to hard fork the chain when errors occur in the communication layer.
5.5.4 How Funds are Handled When Errors Occur During Delegation
It is very important to protect the safety of actors’ funds. As funds need to be transferred across the chain, this BEP will reuse the cross-chain transfer design as before. The key points are that funds transferred from BSC to BC will be locked in the tokenHub system contract and funds transferred from BC to BSC will be locked in the pegAccount. Necessary mechanism is employed to ensure the cross-chain reconciliation.
When a delegation request fails, an extra cross-chain package will be sent back to BSC. The funds will be unlocked from the tokenhub system contract, transferred to the staking system contract and be recorded as a part of the delegator’s pending undelegated. The delegator can get his/her funds back by claimUndelegated later. However, if the msg sender is a contract without a receive/fallback method, the transfer will fail and the funds may be lost forever. So make sure your dApp implements the receive/fallback method.