Skip to content
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

[Staking] Validator gating #1121

Closed
raulk opened this issue Aug 21, 2024 · 4 comments · Fixed by #1127
Closed

[Staking] Validator gating #1121

raulk opened this issue Aug 21, 2024 · 4 comments · Fixed by #1127

Comments

@raulk
Copy link
Contributor

raulk commented Aug 21, 2024

Goals

Introduce the ability to intercept staking, unstaking, and explicit validator membership adjustments (federated membership) to allow or deny the action, according to a user-defined policy.

Proposed solution

  1. Introduce an IValidatorGater standard interface that can be implemented by any contract.
  2. At subnet creation time, add a new parameter to set the address of the gater contract.
  3. If a non-zero address, every action to stake, unstake, and set explicit power should call this contract to determine if the action is allowed or denied. (Callback style).
  4. This should happen at all lifecycle stages of the subnet, whether the subnet is in pre-activation or activated.
  5. The gater should be allowed to call back to the subnet actor or the gateway, e.g. to query the current power table, or else. So reentrancy for read-only operations is fine.

IValidatorGater interface

interface IValidatorGater {
    function interceptStake(SubnetID id, address validator, uint256 amount) returns (bool);

    function interceptUnstake(SubnetID id, address validator, uint256 amount) returns (bool);

    function interceptSetPower(SubnetID id, address validator, uint256 newPower) returns (bool);
}

Implementations

This flexible interface can back any implementation, e.g.:

  • An ACL list.
  • A policy that checks if a given validator is a Filecoin SP, a token holder, an NFT holder, etc.
  • A time-based policy that enables validators to join or leave at specific times.
  • etc.
@cryptoAtwill
Copy link
Contributor

cryptoAtwill commented Aug 22, 2024

@raulk I think it works. Just a small note that do we have to differentiate Stake and Power(I assume Power refers to federated power)? They are all voting power, just how the power is coming from, so I would say interceptIncreasePower, interceptDecreasePower or sth similar should do it. Then it's just the implementation calling these methods as federated power and stake will not be mixed.

@raulk
Copy link
Contributor Author

raulk commented Aug 22, 2024

Unfortunately these have different semantics. stake and unstake are delta-based, whereas setFederatedPower is an override. Perhaps a stronger generalisation might be worthwhile:

interface IValidatorGater {
    function interceptPowerDelta(SubnetID id, address validator, prevPower uint256, newPower uint256) returns (bool);
}

In this case:

  • Staking would be represented as prevPower < newPower.
  • Unstaking would be represented a prevPower > newPower.
  • Override (setFederatedPower) could go in either direction.

We could even make it accept a variadic argument, so we could deal with a batch power change at once, which is useful with setFederatedPower since this way the gater has more information to evaluate the full operation as one, instead of breaking it down into individual changes.

However, I am not sure how shuffling / active validator limits factor in. What are your thoughts on that front? e.g. what happens if a validator stakes but they're not picked as an active validator. Do we still intercept? (I'd say yes)

Another thing I'm not sure about is whether we want a boolean return value, or simply a revert in case of rejection.

@cryptoAtwill
Copy link
Contributor

This interface:

interface IValidatorGater {
    function interceptPowerDelta(SubnetID id, address validator, prevPower uint256, newPower uint256) returns (bool);
}

works for me. I just want to avoid getting into the current stake and federatedPower dual implementation.

As of However, I am not sure how shuffling / active validator limits factor in. What are your thoughts on that front? e.g. what happens if a validator stakes but they're not picked as an active validator. Do we still intercept? (I'd say yes), I think should be no problem with interception, just revert and stake will fail. It makes sense because if the stake is not approve then regardless if it's active validator, it should be rejected.

I prefer a revert over bool as default implementation. If someone needs bool, they can always catch it and return bool. Default revert feels more fiting to the current context.

@raulk
Copy link
Contributor Author

raulk commented Aug 22, 2024

We're aligned, but I'm sure we'll find edge cases during implementation and we may need to send more data to the gater so it can make an informed decision.

I think should be no problem with interception, just revert and stake will fail. It makes sense because if the stake is not approve then regardless if it's active validator, it should be rejected.

To clarify, what I meant is that this interface may get calls for validators that are ultimately not active (either because they don't meet the minimum stake threshold, or because they were shuffled away). I think that's fine; we just need to make sure that the gater can call back to the subnet actor and/or the gateway to query details (i.e. we're not prohibiting all forms of reentrancy).

I prefer a revert over bool as default implementation. If someone needs bool, they can always catch it and return bool. Default revert feels more fiting to the current context.

Who is "they"? It's the IPC contracts that will be interacting with the gater. That said, I think a revert is fine, as a denial would anyway cause a revert during stake/unstake.

@raulk raulk linked a pull request Sep 2, 2024 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants