hats-finance / Accumulated-finance-0x75278bcc0fa7c9e3af98654bce195eaf3bb6a784

0 stars 0 forks source link

Slashed validators can not be removed #24

Open hats-bug-reporter[bot] opened 3 months ago

hats-bug-reporter[bot] commented 3 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xaff8968215019a23ca26d74ec30159659cc015f0f3e632876ddb051e1fda7c0a Severity: high

Description:

Description

function _removeDelegation is used whenever the undelegation process is invoked;

  function takeReceiptUndelegate(uint64 receiptId) public onlyOwner {
        UndelegationReceipt storage receipt = undelegationReceipts[receiptId];
        require(block.number > receipt.blockNumber, "ReceiptNotReady");
        require(receipt.exists == true, "ReceiptNotExists");
        require(receipt.receiptTaken == false, "ReceiptAlreadyTaken");
        (uint64 epoch, uint64 endReceiptId) = Subcall.consensusTakeReceiptUndelegateStart(receiptId);
        Delegation storage d = delegations[receipt.from];

        // update receipt with the necessary info
        receipt.receiptTaken = true;
        receipt.receiptTakenBlockNumber = block.number;
        receipt.epoch = epoch;
        receipt.endReceiptId = endReceiptId;

        // update delegation amount and shares
        d.shares -= receipt.shares;

        if (d.shares == 0) {
=>            _removeDelegation(receipt.from);
        }

        emit TakeReceiptUndelegate(receiptId, epoch, endReceiptId);
    }

This simply removes the validator

The problem however is that a delegator can be slashed as per the documentation:

Keep in mind that the validator's misbehavior will result in slashing or even losing a portion of the staked tokens

If this occurs it means that the validator can not be delegated towards anymore, but since _removeDelegation can only be called inside the undelegate function there currently is no way to remove a validator if it gets slashed.

This can cause for some serious issues since whenever delegating this slashed validator will still be includedwhich can result in a loss of funds or at least issues regarding their funds since this delegator is either now removed or penalized and therefore not trustworthy

Recommendation

introduce a function removeValidator that allows the onlyOwner to remove a validator whenever for example, a slashing happens

ilzheev commented 2 months ago

Shares remains unchanged in case of slashing. https://github.com/hats-finance/Accumulated-finance-0x75278bcc0fa7c9e3af98654bce195eaf3bb6a784/issues/18#issuecomment-2326275119 Validator is removed if all shares are undelegated (remaining shares = 0).