Skip to content

[FIP10] Add new delegation lockin #98

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.

Sign up for GitHub

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

Open
Andrew-Pohl wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[FIP10] Add new delegation lockin #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c2b5727
Add new delegation lockin
Andrew-Pohl Mar 12, 2021
64eb628
fix test
Andrew-Pohl Mar 12, 2021
daa1fd5
Change logic of unbounding period
Andrew-Pohl Sep 30, 2021
3f1f91e
Add withdraw queuing
Andrew-Pohl Oct 19, 2021
ef93e79
Changes to FIP10 flow
Andrew-Pohl Nov 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 90 additions & 1 deletion contracts/ConsensusUtils.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
uint256 public constant CYCLE_DURATION_BLOCKS = 34560; // 48 hours [48*60*60/5]
uint256 public constant SNAPSHOTS_PER_CYCLE = 0; // snapshot each 288 minutes [34560/10/60*5]
uint256 public constant DEFAULT_VALIDATOR_FEE = 15e16; // 15%
uint256 public constant UNBOUNDING_PERIOD = CYCLE_DURATION_BLOCKS;
uint256 public constant MAX_WITHDRAW_QUEUE_LENGTH = 20000;
uint256 public constant MAX_WITHDRAW_CHUNK_SIZE = 100;

/**
* @dev This event will be emitted after a change to the validator set has been finalized
Expand Down Expand Up @@ -110,11 +113,60 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
}
}

function _addToUboundingList(address _staker, uint256 amount) internal {
require(unboundingQueueLength(_staker) < MAX_WITHDRAW_QUEUE_LENGTH);
uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueBlock", _staker))].push(block.number + UNBOUNDING_PERIOD);
uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueAmount", _staker))].push(amount);
}

function unbound(address _staker) internal {
uint256 toReturn = 0;
uint256 currentBlock = block.number;
uint256 index = 0;
uint256 oldArrayLength = unboundingQueueLength(_staker);

//loop through queue and check if we have anything to withdraw
for (uint256 i = 0; i < oldArrayLength; i ++)
{
if (i > MAX_WITHDRAW_CHUNK_SIZE)
Copy link
Contributor

@alonweinstein alonweinstein Jul 8, 2024

Choose a reason for hiding this comment

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

why test this inside the loop?

{
break;
}
if (uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueBlock", _staker))][i] > currentBlock)
{
break;
}
toReturn = toReturn.add(uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueAmount", _staker))][i]);
}

//check if we have anything
require(toReturn != 0);
index = i - 1;
Copy link
Contributor

@alonweinstein alonweinstein Jul 8, 2024

Choose a reason for hiding this comment

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

isn't i scoped to the for loop?


//shift queue and pop back
for (i = 0; i < oldArrayLength - index; i++) {
uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueBlock", _staker))][i] = uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueBlock", _staker))][i + index];
Copy link
Contributor

@alonweinstein alonweinstein Jul 8, 2024

Choose a reason for hiding this comment

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

have you considered the gas costs for all these keccak256 calls? is there a less gas-intensive way to handle this?

uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueAmount", _staker))][i] = uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueAmount", _staker))][i + index];
}
for(i = oldArrayLength - 1; i >= oldArrayLength - index; i--)
{
delete uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueBlock", _staker))][i];
uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueBlock", _staker))].length--;
delete uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueAmount", _staker))][i];
uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueAmount", _staker))].length--;
}

//transfer
_staker.transfer(toReturn);
}

function _withdraw(address _staker, uint256 _amount, address _validator) internal {
require(_validator != address(0));
require(_amount > 0);

require(_amount <= stakeAmount(_validator));
require(_amount <= delegatedAmount(_staker, _validator));
_addToUboundingList(_staker, _amount);

bool _isValidator = isValidator(_validator);

Expand All @@ -139,7 +191,6 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
if (stakeAmount(_validator) < getMinStake()) {
_pendingValidatorsRemove(_validator);
}
_staker.transfer(_amount);
}

function _setSystemAddress(address _newAddress) internal {
Expand Down Expand Up @@ -366,6 +417,10 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
return uintStorage[TOTAL_STAKE_AMOUNT];
}

function unboundingBlock(address _address) public view returns(uint256) {
return uintStorage[keccak256(abi.encodePacked("unboundingBlock", _address))];
}

function _stakeAmountAdd(address _address, uint256 _amount) internal {
uintStorage[keccak256(abi.encodePacked("stakeAmount", _address))] = uintStorage[keccak256(abi.encodePacked("stakeAmount", _address))].add(_amount);
}
Expand All @@ -374,10 +429,22 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
uintStorage[keccak256(abi.encodePacked("stakeAmount", _address))] = uintStorage[keccak256(abi.encodePacked("stakeAmount", _address))].sub(_amount);
}

function _setUnboundingPeriod(address _address) internal {
uintStorage[keccak256(abi.encodePacked("unboundingBlock", _address))] = block.number + UNBOUNDING_PERIOD;
}

function delegatedAmount(address _address, address _validator) public view returns(uint256) {
return uintStorage[keccak256(abi.encodePacked("delegatedAmount", _address, _validator))];
}

function unBoundingAmount(address _address) public view returns(uint256) {
return uintStorage[keccak256(abi.encodePacked("unBoundingAmount", _address))];
}

function _setUnBoundingAmount(address _address, uint256 _amount) internal {
uintStorage[keccak256(abi.encodePacked("unBoundingAmount", _address))] = _amount;
}

function _delegatedAmountAdd(address _address, address _validator, uint256 _amount) internal {
uintStorage[keccak256(abi.encodePacked("delegatedAmount", _address, _validator))] = uintStorage[keccak256(abi.encodePacked("delegatedAmount", _address, _validator))].add(_amount);
if (_address != _validator && !isDelegator(_validator, _address)) {
Expand Down Expand Up @@ -462,6 +529,10 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
return addressArrayStorage[NEW_VALIDATOR_SET].length;
}

function unboundingQueueLength(address _staker) public view returns(uint256) {
return uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueBlock", _staker))].length;
}

function _setNewValidatorSet(address[] _newSet) internal {
addressArrayStorage[NEW_VALIDATOR_SET] = _newSet;
}
Expand Down Expand Up @@ -505,4 +576,22 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
function _setValidatorFee(address _validator, uint256 _amount) internal {
uintStorage[keccak256(abi.encodePacked("validatorFee", _validator))] = _amount;
}

function unboundingAmount() public view returns(uint256) {
Copy link

@sirpy sirpy Nov 2, 2021

Choose a reason for hiding this comment

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

if queue is long this will fail if used inside a smart contract.
we can keep this, but add another view helper canWithdraw() which simply returns true if first in queue can be withdrawn.
so some contract can easily call unbound() then check if still canWithdraw() and call unbound again

I still dont think it is necessary to limit the queue length, as it is per account, that account owner can only attack itself.
for example in our use case, we can simply limit users to withdraw at least 10% of their funds and at least 1 fuse, so they cant create a huge queue withdrawing small amounts

uint256 toReturn = 0;
uint256 currentBlock = block.number;
uint256 oldArrayLength = unboundingQueueLength(msg.sender);

//loop through queue and check if we have anything to withdraw
for (uint256 i = 0; i < oldArrayLength; i ++)
{
if (uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueBlock", msg.sender))][i] > currentBlock)
{
break;
}
toReturn = toReturn.add(uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueAmount", msg.sender))][i]);
}

return toReturn;
}
}
1 change: 1 addition & 0 deletions contracts/interfaces/IConsensus.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ interface IConsensus {
function isFinalized() external view returns(bool);
function stakeAmount(address _address) external view returns(uint256);
function totalStakeAmount() external view returns(uint256);
function unboundingBlock(address _address) external view returns(uint256);
}
33 changes: 31 additions & 2 deletions test/consensus.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const { toBN, toWei, toChecksumAddress } = web3.utils
const MAX_VALIDATORS = 100
const MIN_STAKE_AMOUNT = 10000
const MAX_STAKE_AMOUNT = 50000
const UNBOUNDING_PERIOD = 86400
const MULTIPLY_AMOUNT = 3
const MIN_STAKE = toWei(toBN(MIN_STAKE_AMOUNT), 'ether')
const MAX_STAKE = toWei(toBN(MAX_STAKE_AMOUNT), 'ether')
Expand Down Expand Up @@ -804,6 +805,11 @@ contract('Consensus', async (accounts) => {
it('should not allow more the maximum stake', async () => {
await consensus.stake({from: firstCandidate, value: MORE_THAN_MAX_STAKE}).should.be.rejectedWith(ERROR_MSG)
})

it('unbounding period should remain 0', async () => {
await consensus.stake({from: firstCandidate, value: LESS_THAN_MIN_STAKE}).should.be.fulfilled
ZERO.should.be.bignumber.equal(await consensus.unboundingBlock(firstCandidate))
})
})
describe('advanced', async () => {
it('minimum stake amount, in more than one transaction', async () => {
Expand Down Expand Up @@ -1176,6 +1182,7 @@ contract('Consensus', async (accounts) => {

// await mockEoC()
// withdraw

await consensus.methods['withdraw(uint256)'](MIN_STAKE, {from: firstCandidate})
MIN_STAKE.should.be.bignumber.equal(await web3.eth.getBalance(consensus.address))
ZERO_AMOUNT.should.be.bignumber.equal(await consensus.stakeAmount(firstCandidate))
Expand Down Expand Up @@ -1324,12 +1331,31 @@ contract('Consensus', async (accounts) => {
await consensus.delegate(firstCandidate, {from: firstDelegator, value: MIN_STAKE})
await consensus.methods['withdraw(address,uint256)'](firstCandidate, MORE_THAN_MIN_STAKE, {from: firstDelegator}).should.be.rejectedWith(ERROR_MSG)
})
it('Check unbounding period', async () => {
await consensus.delegate(firstCandidate, {from: firstDelegator, value: MAX_STAKE})
let currentBlockNumber = await web3.eth.getBlockNumber()
//check unbounding has updated
toBN((UNBOUNDING_PERIOD + currentBlockNumber)).should.be.bignumber.equal(await consensus.unboundingBlock(firstDelegator))
await consensus.methods['withdraw(address,uint256)'](firstCandidate, MIN_STAKE, {from: firstDelegator}).should.be.rejectedWith(ERROR_MSG)
await advanceBlocks(UNBOUNDING_PERIOD)
await consensus.methods['withdraw(address,uint256)'](firstCandidate, MIN_STAKE, {from: firstDelegator}).should.be.fulfilled
//check a second withdraw
await consensus.methods['withdraw(address,uint256)'](firstCandidate, MIN_STAKE, {from: firstDelegator}).should.be.fulfilled
let oldunBound = await consensus.unboundingBlock(firstDelegator)
//check for updated
await consensus.delegate(firstCandidate, {from: firstDelegator, value: MIN_STAKE}).should.be.fulfilled
oldunBound.should.be.bignumber.below(await consensus.unboundingBlock(firstDelegator))
await consensus.methods['withdraw(address,uint256)'](firstCandidate, MIN_STAKE, {from: firstDelegator}).should.be.rejectedWith(ERROR_MSG)
await advanceBlocks(UNBOUNDING_PERIOD)
await consensus.methods['withdraw(address,uint256)'](firstCandidate, MIN_STAKE, {from: firstDelegator}).should.be.fulfilled
})
it('can withdraw all staked amount', async () => {
// stake
await consensus.delegate(firstCandidate, {from: firstDelegator, value: MIN_STAKE})
// stake
await consensus.delegate(secondCandidate, {from: firstDelegator, value: MIN_STAKE})
// withdraw
//advance block and withdraw
await advanceBlocks(UNBOUNDING_PERIOD)
await consensus.methods['withdraw(address,uint256)'](firstCandidate, MIN_STAKE, {from: firstDelegator})
MIN_STAKE.should.be.bignumber.equal(await web3.eth.getBalance(consensus.address))
ZERO_AMOUNT.should.be.bignumber.equal(await consensus.stakeAmount(firstCandidate))
Expand All @@ -1353,9 +1379,11 @@ contract('Consensus', async (accounts) => {
firstDelegator.should.be.equal(await consensus.delegatorsAtPosition(secondCandidate, 0))
})
it('can withdraw less than staked amount', async () => {

// stake
await consensus.delegate(firstCandidate, {from: firstDelegator, value: MIN_STAKE})
// withdraw
//advance block and withdraw
await advanceBlocks(UNBOUNDING_PERIOD)
await consensus.methods['withdraw(address,uint256)'](firstCandidate, ONE_ETHER, {from: firstDelegator})
let expectedAmount = toWei(toBN(MIN_STAKE_AMOUNT - 1), 'ether')
let expectedValidators = []
Expand All @@ -1374,6 +1402,7 @@ contract('Consensus', async (accounts) => {
it('can withdraw multiple times', async () => {
// stake
await consensus.delegate(firstCandidate, {from: firstDelegator, value: MIN_STAKE})
await advanceBlocks(UNBOUNDING_PERIOD)
// withdraw 1st time
await consensus.methods['withdraw(address,uint256)'](firstCandidate, ONE_ETHER, {from: firstDelegator})
let expectedAmount = toWei(toBN(MIN_STAKE_AMOUNT - 1), 'ether')
Expand Down