keep-core icon indicating copy to clipboard operation
keep-core copied to clipboard

Allowlist for Wallet Registry

Open pdyraga opened this issue 10 months ago • 0 comments

The allowlist contract replaces the Threshold TokenStaking contract and is as an outcome of TIP-092 and TIP-100 governance decisions. Staking tokens is no longer required to operate nodes. Beta stakers are selected by the DAO and operate the network based on the allowlist maintained by the DAO. The contract will be integrated with the WalletRegistry and replace calls to TokenStaking.

I have been experimenting with various approaches, and the most extreme one was to remove most of the EcdsaAuthorization logic as well as all TokenStaking.seize calls. This would have cascading effects on tBTC Bridge contracts as they rely on WalletRegistry.seize. That would also require implementing weight decrease delays in the Allowlist, so essentially doing work that is already done in WalletRegistry. Considering the pros and cons, I decided on the least invasive option. The WalletRegistry still thinks in terms of stake authorization, but everything is based on the staking provider's weight as set in the Allowlist, and weight decrease delays are enforced by the existing mechanism in EcdsaAuthorization. The seize function does nothing except of emitting an event about detecting beta staker misbehavior.

To be done

Deployment script

We need to capture all existing beta stakers along with their current authorizations and initialize the Allowlist contract. We can do it by either replicating the existing weights or giving them all the same weight.

Integrate with WalletRegistry and tests

There are two approaches to achieve it. The first one is to get rid of all references to TokenStaking from tests and update them to work with Allowlist. Another approach is to let them work with TokenStaking but introduce another integration test for those two contracts. In this option, we could use in WalletRegistry something like:

    modifier onlyStakingContract() {
        address _allowlist = address(allowlist);
        require(
            // If the allowlist is set, accept calls only from the allowlist.
            // This is post-TIP-98 scenario. If the allowlist is not set, accept
            // calls only from the staking contract. This is pre-TIP-98 scenario.
            (_allowlist != address(0) && msg.sender == _allowlist) ||
                (_allowlist == address(0) && msg.sender == address(staking)),
            "Caller is not the staking contract"
        );
        _;
    }

    /// @notice Initializes V2 version of the WalletRegistry operating with the
    ///         Allowlist contract, as a result of TIP-098 and TIP-100 governance
    ///         decisions.
    function initializeV2(address _allowlist) external reinitializer(2) {
        allowlist = Allowlist(_allowlist);
    }

    /// @dev Provides the expected IStaking reference. If the allowlist is set,
    ///      it acts as the staking contract. If it is not set, the TokenStaking
    ///      acts as the staking contract.
    function _staking() internal returns (IStaking) {
        if (address(allowlist) != address(0)) {
            return IStaking(allowlist);
        }

        return staking;
    }

Note that the WalletRegistry is close to the maximum allowed contract size and - surprise! - adding the logic above makes it exceed the allowed size. This could potentially be alleviated by removing some of the functionality. For example, in the challengeDkgResult function we have a try catch as well as a call to dkg.requireChallengeExtraGas(). This could potentially be eliminated as a no-op seize in Allowlist is guaranteed to always succeed. Also, post EIP-7702, the require(msg.sender == tx.origin, "Not EOA") check is no longer guaranteed to work as expected.

pdyraga avatar Mar 05 '25 12:03 pdyraga