slashing: add proof account functionality #7394

AshwinSekar · 2024-10-24T16:50:06Z

Initial skeleton of slashing program.

Adds proof account creation, write and close, mostly plagiarized from the record program.
Since proof sizes are known upfront there is no need to realloc.

Future PR will add verification for this proof.

AshwinSekar · 2024-10-24T16:51:00Z

slashing/program/program-id.md

@@ -0,0 +1 @@
+8sT74BE7sanh4iT84EyVUL8b77cVruLHXGjvTyJ4GwCe


Just used the default key when from when I deployed to testnet, can update once we decide how this is going to be deployed.

No problem. Since this will be an "enshrined" program we could even add it a non-random address with a feature-gate, like S1ashing1111111111111....

Got it, let me know if I have this correct for the deployment scheme:

We upload the program to some address A

On epoch boundary where the feature flag is active we create a program account owned by the system program S1ashing1111...

We them move the contents of A to this vanity address

Further upgrades to the program will require another feature flag

I did see that some of the other spl programs were deployed with a squads multi sig as the upgrade authority. Curious about your thoughts about that deployment schema.

It seems that would allow us more freedom in upgrading or disabling the slashing program should there be any vulnerabilities. But optically is it worse if the keys are held by anza & FD?

Yeah, that would be the process. Anza / FD really can't hold the upgrade keys to something so sensitive -- we'll need to leave it up to the validators. We can start off with feature gates, and then eventually move to a validator DAO, if that ever materializes.

AshwinSekar · 2024-10-24T16:52:44Z

slashing/program/src/processor.rs

+                return Err(ProgramError::InvalidAccountData);
+            }
+
+            msg!("Creating proof account with size {}", account_length);


I provided the functionality for the user to create and initialize the account here, but if you prefer we could have them create the account and write it themselves using the record program, only interacting with slashing for the verification.

Not sure what the preferred style is for these things.

That's a good question. If these proofs might be really big, we could go with the same model as zk proofs, where the proofs can be read from instruction data or account data (doesn't matter which program).

This way, the slashing program gets way simpler -- it just needs to take in data, check it, and then create a valid slash "context state account" from that, containing the minimum required data, ie:

validator that committed the slashable offense

pubkey that submitted the proof

kind of slashable offense

epoch in which it happened

These would look similar to the "context state accounts" in the zk elgamal proof program. Here's how the program looks: https://github.com/anza-xyz/agave/blob/master/programs/zk-elgamal-proof/src/lib.rs

And the ZkProofData trait: https://github.com/anza-xyz/agave/blob/master/zk-sdk/src/zk_elgamal_proof_program/proof_data/mod.rs#L65

And one example separating the proof data from context: https://github.com/anza-xyz/agave/blob/c88e6df566c5c17d71e9574785755683a8fb033a/zk-sdk/src/zk_elgamal_proof_program/proof_data/pubkey_validity.rs#L35

The only issue with using account data owned by any program is that the data could get changed afterwards, which can make auditability more difficult. The slashing program could get around that by using the same trick as account-compression, where the program CPIs into a no-op program with all of the proof data, which forces the runtime to log the instruction data. This way it's stored longterm.

Or, keep it simple, and make the "context state account" contain all the data.

Does this make sense? What do you think?

That's fair, I am not opposed to having the user create and write the proof data themselves.
To answer your questions:

The proof sizes will be fairly large. I expect the duplicate block proof to be the smallest with ~2400 bytes

For now I expect the proofs to be of fixed size known up front for each proof type, although this might change if we decide to add more slashing conditions in the future

There also should be no reason to CPI into the slashing program, or create the proof on chain. All slashable events occur (at least partially) offchain by nature.

I haven't decided yet on how the results will be collected but the context state account makes sense. At most we'd need 1 account per staked validator.
We could also just maintain a more compact structure instead of creating a separate account: HashMap<Epoch, (Pubkey, Vec<ProofType, usize>>> essentially a list of # of infractions by type per pubkey.

Gotcha, that's all good context. A few followups:

There also should be no reason to CPI into the slashing program, or create the proof on chain. All slashable events occur (at least partially) offchain by nature.

You never know what people will do! Just so I'm sure I'm not totally misunderstanding, when you say "no reason to create the proof on chain", you mean specifically "from another program", correct? Because the concept is that someone will upload the whole proof on-chain, right?

I haven't decided yet on how the results will be collected but the context state account makes sense. At most we'd need 1 account per staked validator.

Ah right, yeah that makes sense. I was thinking (naively) that we wouldn't even need accounts per validator, since you can just submit proofs as individual accounts, but we probably want to avoid duplicate submissions. For example, if a validator creates a duplicate block, and they created more than one pair of offending shreds, then there will be two distinct valid slash proofs, but for the same event.

Ok, so then we'll need two account types: proofs (can be created on any address) and validator accounts (some PDA that can be instantiated by anyone). Let's avoid the hashmap -- that can become an easy attack vector.

you mean specifically "from another program", correct

Yeah exactly, the proof will still be uploaded on-chain.

For example, if a validator creates a duplicate block, and they created more than one pair of offending shreds, then there will be two distinct valid slash proofs, but for the same event.

This is a good point, avoiding duplicates definitely makes it worth tracking per validator. I was also thinking that since the runtime code will eventually have to traverse these accounts in order to debit delegations so it's a good idea to keep it bounded.

Let's avoid the hashmap -- that can become an easy attack vector.

Sounds good.

joncinque

Looks like a good start! Let's figure out how this program should operate, and then we can dig down more into the implementation.

joncinque · 2024-10-28T20:14:45Z

slashing/program/Xargo.toml

Nit: this file hasn't been needed for a long time, so we can remove it

joncinque · 2024-10-28T20:16:21Z

slashing/program/program-id.md

@@ -0,0 +1 @@
+8sT74BE7sanh4iT84EyVUL8b77cVruLHXGjvTyJ4GwCe


No problem. Since this will be an "enshrined" program we could even add it a non-random address with a feature-gate, like S1ashing1111111111111....

joncinque · 2024-10-28T20:19:16Z

slashing/program/src/error.rs

+    /// Incorrect authority provided on write or close
+    #[error("Incorrect authority provided on write or close")]
+    IncorrectAuthority,


nit: we could use ProgramError::IncorrectAuthority instead, but I don't feel too strongly about it

joncinque · 2024-10-28T20:19:32Z

slashing/program/src/error.rs

+    /// Calculation overflow
+    #[error("Calculation overflow")]
+    Overflow,


nit: we could use ProgramError::ArithmeticOverflow instead

joncinque · 2024-10-28T21:01:59Z

slashing/program/src/processor.rs

+                return Err(ProgramError::InvalidAccountData);
+            }
+
+            msg!("Creating proof account with size {}", account_length);


That's a good question. If these proofs might be really big, we could go with the same model as zk proofs, where the proofs can be read from instruction data or account data (doesn't matter which program).

This way, the slashing program gets way simpler -- it just needs to take in data, check it, and then create a valid slash "context state account" from that, containing the minimum required data, ie:

validator that committed the slashable offense

pubkey that submitted the proof

kind of slashable offense

epoch in which it happened

These would look similar to the "context state accounts" in the zk elgamal proof program. Here's how the program looks: https://github.com/anza-xyz/agave/blob/master/programs/zk-elgamal-proof/src/lib.rs

And the ZkProofData trait: https://github.com/anza-xyz/agave/blob/master/zk-sdk/src/zk_elgamal_proof_program/proof_data/mod.rs#L65

And one example separating the proof data from context: https://github.com/anza-xyz/agave/blob/c88e6df566c5c17d71e9574785755683a8fb033a/zk-sdk/src/zk_elgamal_proof_program/proof_data/pubkey_validity.rs#L35

The only issue with using account data owned by any program is that the data could get changed afterwards, which can make auditability more difficult. The slashing program could get around that by using the same trick as account-compression, where the program CPIs into a no-op program with all of the proof data, which forces the runtime to log the instruction data. This way it's stored longterm.

Or, keep it simple, and make the "context state account" contain all the data.

Does this make sense? What do you think?

joncinque · 2024-10-28T21:06:30Z

slashing/program/src/state.rs

+/// Types of slashing proofs
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum ProofType {
+    /// Proof consisting of 2 shreds signed by the leader indicating the leader
+    /// submitted a duplicate block.
+    DuplicateBlockProof,
+    /// Invalid proof type
+    InvalidType,
+}


I would start with 0 as invalid, since accounts are initialized with all zeroes.

joncinque · 2024-10-28T21:07:19Z

slashing/program/src/state.rs

+    /// Struct version, allows for upgrades to the program
+    pub version: u8,


We probably don't need versioning for this -- we would likely create new proof types and deprecate the old ones as needed

AshwinSekar · 2024-10-30T21:19:38Z

per the comments above, closing this in favor of #7418

mergify bot added the community Community contribution label Oct 24, 2024

AshwinSekar commented Oct 24, 2024

View reviewed changes

slashing: add proof account functionality

9be74cb

AshwinSekar force-pushed the slashing branch from 037cda5 to 9be74cb Compare October 24, 2024 17:12

AshwinSekar requested a review from joncinque October 24, 2024 17:51

joncinque reviewed Oct 28, 2024

View reviewed changes

AshwinSekar closed this Oct 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

slashing: add proof account functionality #7394

slashing: add proof account functionality #7394

AshwinSekar commented Oct 24, 2024 •

edited

Loading

AshwinSekar Oct 24, 2024

joncinque Oct 28, 2024

AshwinSekar Oct 28, 2024

joncinque Oct 28, 2024

AshwinSekar Oct 24, 2024 •

edited

Loading

joncinque Oct 28, 2024

AshwinSekar Oct 28, 2024

joncinque Oct 28, 2024 •

edited

Loading

AshwinSekar Oct 28, 2024 •

edited

Loading

joncinque left a comment

joncinque Oct 28, 2024

joncinque Oct 28, 2024

joncinque Oct 28, 2024

joncinque Oct 28, 2024

joncinque Oct 28, 2024

joncinque Oct 28, 2024

joncinque Oct 28, 2024

AshwinSekar commented Oct 30, 2024

		/// Struct version, allows for upgrades to the program
		pub version: u8,

		@@ -0,0 +1 @@
		8sT74BE7sanh4iT84EyVUL8b77cVruLHXGjvTyJ4GwCe

		@@ -0,0 +1 @@
		8sT74BE7sanh4iT84EyVUL8b77cVruLHXGjvTyJ4GwCe

slashing: add proof account functionality #7394

slashing: add proof account functionality #7394

Conversation

AshwinSekar commented Oct 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AshwinSekar Oct 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joncinque Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

AshwinSekar Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

joncinque left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AshwinSekar commented Oct 30, 2024

AshwinSekar commented Oct 24, 2024 •

edited

Loading

AshwinSekar Oct 24, 2024 •

edited

Loading

joncinque Oct 28, 2024 •

edited

Loading

AshwinSekar Oct 28, 2024 •

edited

Loading