Skip to content

Conversation

just-mitch
Copy link
Collaborator

No description provided.

@aminsammara
Copy link
Contributor

aminsammara commented May 7, 2025

There are some marked changes from what we agreed on earlier today:

  1. The slashing amount was variable when we spoke. It was confirmed that each payload can specify the amount to be slashed. This design does not accommodate that.

  2. Slashing could be for any reason not just missed attestations. The example we discussed was slashing a validator because they're from NK - if validators so choose to do so.
    -> Can this be done by having node operators select 0% missed attestations as the threshold for auto-voting yes? So their nodes can slash another validator with perfect uptime?

  3. What was not yet finalized in our call and is missing here is the MIN_STAKE_TO_STAY_IN_SET=/=MIN_STAKE_TO_ENTER_SET discussion.

@just-mitch
Copy link
Collaborator Author

There are some marked changes from what we agreed on earlier today:

  1. The slashing amount was variable when we spoke. It was confirmed that each payload can specify the amount to be slashed. This design does not accommodate that.
  2. Slashing could be for any reason not just missed attestations. The example we discussed was slashing a validator because they're from NK - if validators so choose to do so.
    -> Can this be done by having node operators select 0% missed attestations as the threshold for auto-voting yes? So their nodes can slash another validator with perfect uptime?
  3. What was not yet finalized in our call and is missing here is the MIN_STAKE_TO_STAY_IN_SET=/=MIN_STAKE_TO_ENTER_SET discussion.
  1. Yes, it does. It just isn't convenient. This contract is out of protocol. Anyone else could deploy a new instance with a different amount to slash for. It is much easier to get everyone to agree to slash if the amount is fixed. If the punishment is wrong, we can deploy a new contract. Generally I fail to see why the punishment for the same crime should be different from time to time, or at least why we should build that complexity immediately.
  2. Note the sentence from the doc "we could update the staking lib to allow the governance (not just the slasher) to slash validators; then we could "propose with lock" to slash whatever validators governance decides.". I'd like to have this "custom slash factory" be used for automatic/explicit cases. Given that, changing the (in-protocol) slasher to listen to governance as well should be done in a different work stream.
  3. Seems entirely independent from the work at hand. I'd rather not address it here.

@aminsammara
Copy link
Contributor

aminsammara commented May 7, 2025

Thanks for clarifying Mitch.

or at least why we should build that complexity immediately

These contracts are hopefully what we send to audit and ship on Ignition. I'm under the understanding that we don't want to iterate on this.

Note the sentence from the doc "we could update the staking lib to allow the governance (not just the slasher) to slash validators; then we could "propose with lock" to slash whatever validators governance decides."

Requiring propose with lock to do this is very unpleasant. I'll talk toJoe about whether we can even do this in Ignition. What are reasons for not allowing validators to vote on arbitrary slashing? i.e. without explicitly checking for missed attestations?

Minor / resolved comments

Yes, it does. It just isn't convenient. This contract is out of protocol. Anyone else could deploy a new instance with a different amount to slash for. It is much easier to get everyone to agree to slash if the amount is fixed. If the punishment is wrong, we can deploy a new contract.

Okay so if anyone can deploy a new contract to create slashing payloads that slash 1% and then another one to create payloads that slash some different x% then sounds fine. We can deploy a bunch of them at 1% 15% and 100% and nodes can be configured to vote for payloads from any of those CustomSlashFactory.

Seems entirely independent from the work at hand. I'd rather not address it here.

I do not agree (due to capital efficiency and fault tolerance) but I'm not too bothered by it. As long as the cost of slashing is high, slashing to evict from the set is the most common use case. As long as variable amounts are possible, we can slash for 0.01% and evict non-malicious inactive validators from the set.

@just-mitch
Copy link
Collaborator Author

If it is a MUST requirement, I'll make the amount to slash configurable per offense without needing different factories. This will require some additional sorting logic on the node, to make sure that you can still come to consensus independently. Assuming people can adjust their thresholds, it also risks us never auto-slashing because of people using different configurations, which is why I am against it; I would say just slash 100% on testnet, and slash 1% on mainnet, and avoid the complexity: if the market demands different parameters, they can deploy it.

If it is a MUST requirement, I'll add an "other" offense which will not have any correspondent module to "verify" on the node- the only way people will vote for it is if they configure a specific slashing payload.

If it is a MUST requirement, I'll add a line about supporting a different value for "enter" versus "stay" stake amounts.

@iAmMichaelConnor
Copy link
Contributor

iAmMichaelConnor commented May 8, 2025

Are these new slashing conditions (eg missing 90% of attestations) demonstrable to a smart contract? Or can validators vote to slash anyone without any demonstrable evidence? Maybe the answer lies in my next Q: What's the Sentinel?

(I'm trying to understand if this proposal enables someone to be slashed without any evidence, just from a vote of a small group of nodes, which would be bad).

@just-mitch
Copy link
Collaborator Author

(I'm trying to understand if this proposal enables someone to be slashed without any evidence, just from a vote of a small group of nodes, which would be bad).

We do have "social slashing": you don't need to prove anything to the L1 other than a configurable number of votes, M from sequencers within a round of N L2 slots. Presently in testnet, M is 6, N is 10. Presumably on mainnet that will be more like 67/100.

We had a long discussion yesterday with @aminsammara who was suggesting to have configurable/dyanmic M and N and penalties based on the alleged offense, e.g. in some cases allowing 33/100 to slash validators for a small amount, which @LHerskind , @Maddiaa0 and I resisted for a few reasons: that makes it pretty easy for a malicious minority to slowly slash everyone else; in a system that has basic sybil resistance, we ought to always have our "honest majority assumption" that 2/3 + 1 of the validator set is honest, so you should always be able to use high M/N; the Slasher contract (which is in-protocol) will execute whatever payload satisfies M/N, so anyone can deploy a slashing payload penalizing arbitrary validator(s) and any arbitrary amount and try to convince the validators to vote for it.

We currently have one module that is capable of utilizing slashing, which slashes all the validators in an epoch if the epoch was not proven, which in essence serves two purposes: the validators should not commit things which are unprovable/incorrect AND they should make the data available to provers.

This new line of work is to add a new factory/payload to L1 which validators can use to slash specific validators for specific amounts for inactivity as reported by the sentinel, which is a module in the node which monitors the p2p network and detects the percentage of times that a validator who could produce an attestation did.

So yes, the evidence is always computed off-chain, but it is currently always the case that M/N is a majority.

@iAmMichaelConnor
Copy link
Contributor

iAmMichaelConnor commented May 8, 2025

With no on-chain evidence, the network could end up in nasty situations where someone is being vote-slashed because of who they are offchain, rather than their demonstrable on-chain actions (or inactions). It opens up some difficult political questions.

It seems reasonable that an honest majority of chosen validators would adhere to the rules of the blockchain and attest to having validated and seen block data. But I'm not sure that same assumption can be applied to wider offchain political motivations to vote to slash someone.

You could have a cohort of validators that agree to run the blockchain correctly (so that they collectively don't get slashed), but they agree to brigade and vote-slash a particular validator if they ever get the chance. The old "F that guy in particular".

To put it another way: An honest majority of people might agree that they (honestly!) dislike someone. I can think of many situations where >67% of people would agree in their dislike of someone.

@LHerskind
Copy link
Contributor

With no on-chain evidence, the network could end up in nasty situations where someone is being vote-slashed because of who they are offchain, rather than their demonstrable on-chain actions (or inactions). It opens up some difficult political questions.

image

The reason it was decided was that plenty of the things that can get you slashed don't leave much trace on L1 that is not possible to manipulate by the same set of actors.

Lets say that we want to slash someone because they are not attesting or proposing blocks. If you rely on the on-chain data, you cannot distinguish between:

  • Alice is offline
  • People don't like Alice, so they do not attest to her blocks and they ignore her attestion

In both cases, on-chain you just see lack of things from Alice, so if it is inactivity you want to slash for, you slash her.


## Executive Summary

The L1 contracts currently only "allow" slashing all validators in an epoch if the epoch is never proven.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not true.

The L1 contract allow any kind of slash that is address,amount pair based, BUT we only have a "detector" working for the epoch failed to be proven one.

There is an important distinction.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks. Was being blunt here for "executive summary". I will clarify.

- How/where to vote/signal on L1
- [x] Node operators SHOULD be able to configure their node to specify thresholds for what they consider "not participating".
- [x] The "offence" that triggers the slash MAY be specified on L1.
- [ ] The amount to be slashed MAY be configurable without deploying a new contract.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just pointing out here that NO slashing in our setup can happen without deploying as it is needing the payload to be deployed. I believe you might be talking about without deploying a new factory?

Unless you have a proxy or updatable payload this will be the case. And lord please smite anyone doing that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, I will clarify that I mean without deploying a new factory.


## Overview

Rename the existing `SlashFactory` contract to `EpochSlashFactory` to denote that it slashes all validators in an epoch. Make a new `CustomSlashFactory` contract which creates a payload to slash a provided list of validators for an explicit offence. The amounts to be slashed for each offence will be provided in the constructor of the `CustomSlashFactory` contract. There will only be one explicit offence right now: "missing attestations". Creating the payload via the `CustomSlashFactory` will emit an event with the payload address, and the validator addresses.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not nuke the current factory? Slashing the members of a committe is just slashing a list of addresses, which is the same as for missing attestations just with a different "type" 🤷. No need to have an extra one just to have it.

Copy link
Collaborator Author

@just-mitch just-mitch May 8, 2025

Choose a reason for hiding this comment

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

Yep, I thought about it, but I figured this is a smaller change, and since we will probably want an "address book", having yet another address that nodes want to keep track of is less onerous. Happy to go either way.

Edit: your comment below on simplifying the ordering heuristic tipped me in favor of having a single factory 👍


## Overview

Rename the existing `SlashFactory` contract to `EpochSlashFactory` to denote that it slashes all validators in an epoch. Make a new `CustomSlashFactory` contract which creates a payload to slash a provided list of validators for an explicit offence. The amounts to be slashed for each offence will be provided in the constructor of the `CustomSlashFactory` contract. There will only be one explicit offence right now: "missing attestations". Creating the payload via the `CustomSlashFactory` will emit an event with the payload address, and the validator addresses.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not nuke the current factory? Slashing the members of a committe is just slashing a list of addresses, which is the same as for missing attestations just with a different "type" 🤷. No need to have an extra one just to have it.

Internally, though, the SlasherClient will monitor two conditions:

- an epoch was not proven, so slash all validators via the `EpochSlashFactory` contract (i.e. the current state, rename the L1 contract from `SlashFactory`)
- a validator has missed X% (e.g. 90%) of attestations according to the Sentinel, so slash that validator via the `CustomSlashFactory` contract (this is new)
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, would just have one path. Simpler to deal with.

- an epoch was not proven, so slash all validators via the `EpochSlashFactory` contract (i.e. the current state, rename the L1 contract from `SlashFactory`)
- a validator has missed X% (e.g. 90%) of attestations according to the Sentinel, so slash that validator via the `CustomSlashFactory` contract (this is new)

Validators will need to have a way to order the various slashing events they observe.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest a different sorting.

Namely, alter the custom such that the event you emit is something like

event PayloadCreated(
    address payload,
    address[] memory attesters, 
    uint256[] memory amounts, 
    uint256[] memory offense,
    uint256 totalSlash
);

Then rely on the same factory for any kind of slash, e.g., missing attestations or whatever.

Have some idea of a deadline, e.g., how long since the payload was created will you still want to vote for it.

First your filter, a "candidate" payload is:

  • alive, current time is before its deadline
  • consistent with your view (your nodes state)

Then you sort by totalSlash and take the highest value. At this point, you cast votes for the biggest slash that you agree with, that you believe is not yet stale.


To reduce overhead, it is possible to just emit an event with the payload address and and then retrieve the inputs from the tx calldata.


The amount to slash should be high for testnet (e.g. the minimum stake amount). We can use a different amount for mainnet.

The threshold of number of validators (M/N) that need to signal/vote for the CustomSlashFactory payload will be the same as the number of validators that need to signal/vote for the EpochSlashFactory payload.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just making clear, that it is not the factories that are specifying N and M, it is the slashing proposer.

@LHerskind
Copy link
Contributor

As @just-mitch I think the enter vs stay amounts don't belong here. Am already having something for it in the GSE so also simpler if needed that we keep it out from here. As I see it, this is really just the how to coordinate the payload

chore: refactor to have a single slash factory
chore: update summary, more detail in implementation
Copy link
Contributor

@LHerskind LHerskind left a comment

Choose a reason for hiding this comment

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

Two small comments and a nit think it is in a good state.

Copy link
Contributor

Choose a reason for hiding this comment

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

The file name is still called .template, probably should be renamed.

## Overview
## L1 Changes

We make no changes to the `Slasher` contract, or any other "in-protocol" contracts.
Copy link
Contributor

Choose a reason for hiding this comment

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

If we really want to support the "configurable" you can do that by making the slasher updatable by either the slasher itself or governance. Fairly minimal change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, I'll mention it as a possibility.


```solidity
interface ISlashFactory {
enum Offense {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would NOT use an enum, as it is then specific to what we conjured up now, I would just use a number, such that it can be handled "off-chain" to map it to something specific.


The contract used as the slasher is currently `Slasher`, which takes directives from a `SlashingProposer`.

The `SlashingProposer` is an instance of `EmpireBase`, which operates in "rounds" of `N` L2 slots, during which at least `M` proposers must vote for a specific contract address "payload" to be executed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The `SlashingProposer` is an instance of `EmpireBase`, which operates in "rounds" of `N` L2 slots, during which at least `M` proposers must vote for a specific contract address "payload" to be executed.
The `SlashingProposer` is an instance of `EmpireBase`, which operates in "rounds" of `M` L2 slots, during which at least `N` proposers must vote for a specific contract address "payload" to be executed.

super nit, but suggesting to keep N, and M similar to governance proposer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not a super nit! I mixed them up. They should just be properly named in the contract.

- for each slot, call `Sentinel.processSlot` to get a map of validators and whether they voted
- emit a `WANT_TO_SLASH` event for each validator that missed more than `SLASH_INACTIVITY_CREATE_TARGET` slots, slashing them for the amount specified in `SLASH_INACTIVITY_CREATE_PENALTY`

When asked, it will agree to slash any validator that missed more than `SLASH_INACTIVITY_SIGNAL_TARGET` slots.
Copy link
Contributor

@aminsammara aminsammara May 14, 2025

Choose a reason for hiding this comment

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

Should the nodes set a SLASH_INACTIVITY_SIGNAL_PENALTY? So a node doesn't just accept an arbitrary SLASH_INACTIVITY_CREATE_PENALTY that was sent to it?

Especially if sorted by highest amount, a node could always agree to slash non-attesters for the highest amount proposed (by any number of WANT_TO_SLASH events).

Copy link
Contributor

Choose a reason for hiding this comment

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

Another idea is an array of SLASH_INACTIVITY_SIGNAL_TARGET and the corresponding array of SLASH_INACTIVITY_SIGNAL_PENALTY

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I'd keep it simple with just a SLASH_INACTIVITY_MAX_PENALTY- don't sign or create anything greater than that.


A `InvalidBlockWatcher` will take an executor as an argument, subscribe to the `invalid-block` event, and then emit a `WANT_TO_SLASH` event naming the proposer of the invalid block, slashing them for the amount specified in `SLASH_INVALID_BLOCK_PENALTY`.

When asked, it will agree to slash any validator that proposed an invalid block which it sees in its cache of invalid blocks.
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar thing where a max penalty a node is willing to accept is helpful.

- `SLASH_OVERRIDE_PAYLOAD`: the address of a payload to signal/vote for no matter what
- `SLASH_PRUNE_ENABLED`: whether to create a payload for epoch pruning
- `SLASH_PRUNE_PENALTY`: the amount to slash each validator that was in an epoch that was pruned
- `SLASH_INACTIVITY_CREATE_TARGET`: the percentage of attestations missed required to create a payload
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any good in including a SLASH_INACTIVITY_ENABLED flag?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, we should have it. Thanks!

@just-mitch just-mitch requested a review from aminsammara May 19, 2025 23:41
@just-mitch just-mitch merged commit 3e4d81b into main May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants