Skip to content
Open
Changes from 1 commit
Commits
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
25 changes: 24 additions & 1 deletion contracts/ConsensusUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
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 = 500;
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 @@ -127,6 +128,10 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
//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
Copy Markdown
Contributor

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;
Expand Down Expand Up @@ -571,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
Copy Markdown

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;
}
}