From 2892c05157b456d7bfc4e3141df6ef18efc0c688 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Tue, 17 Dec 2024 16:17:44 -0500 Subject: [PATCH 1/2] On-chain unresponsiveness reporting (#1215) * Add extrinsic to report unstable peer * Handle errors properly * Add a couple of reporting tests * RustFmt * Bump metadata * Add benchmark for `report_unstable_peer` * Run benches for `report_unstable_peer` * Use benchmark result in pallet * Add doccomment to extrinsic * Add Slashing pallet to Propagation mock * Actually should probably be a `dev-dependency` * Add Slashing pallet to Attestation mock * Add Slashing pallet to Registry mock * TaploFmt * Fix typo * Add `CHANGELOG` entry --- CHANGELOG.md | 1 + Cargo.lock | 4 ++ crates/client/entropy_metadata.scale | Bin 209990 -> 210103 bytes pallets/attestation/Cargo.toml | 2 + pallets/attestation/src/mock.rs | 13 +++++ pallets/propagation/Cargo.toml | 5 +- pallets/propagation/src/mock.rs | 13 +++++ pallets/registry/Cargo.toml | 1 + pallets/registry/src/benchmarking.rs | 2 +- pallets/registry/src/mock.rs | 13 +++++ pallets/staking/Cargo.toml | 2 + pallets/staking/src/benchmarking.rs | 42 +++++++++++++ pallets/staking/src/lib.rs | 53 +++++++++++++++++ pallets/staking/src/mock.rs | 38 ++++++++++++ pallets/staking/src/tests.rs | 55 ++++++++++++++++++ pallets/staking/src/weights.rs | 37 ++++++++++++ .../src/weights/pallet_staking_extension.rs | 18 ++++++ 17 files changed, 296 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95388bab7..efa13cc42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ runtime - Add TSS endpoint to get TDX quote ([#1173](https://github.com/entropyxyz/entropy-core/pull/1173)) - Add TDX test network chainspec ([#1204](https://github.com/entropyxyz/entropy-core/pull/1204)) - Test CLI command to retrieve quote and change endpoint / TSS account in one command ([#1198](https://github.com/entropyxyz/entropy-core/pull/1198)) +- On-chain unresponsiveness reporting [(#1215)](https://github.com/entropyxyz/entropy-core/pull/1215) ### Changed - Use correct key rotation endpoint in OCW ([#1104](https://github.com/entropyxyz/entropy-core/pull/1104)) diff --git a/Cargo.lock b/Cargo.lock index 384f1fa8c..4e486e848 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6768,6 +6768,7 @@ dependencies = [ "pallet-balances", "pallet-parameters", "pallet-session", + "pallet-slashing", "pallet-staking", "pallet-staking-extension", "pallet-staking-reward-curve", @@ -7287,6 +7288,7 @@ dependencies = [ "pallet-programs", "pallet-registry", "pallet-session", + "pallet-slashing", "pallet-staking", "pallet-staking-extension", "pallet-staking-reward-curve", @@ -7353,6 +7355,7 @@ dependencies = [ "pallet-parameters", "pallet-programs", "pallet-session", + "pallet-slashing", "pallet-staking", "pallet-staking-extension", "pallet-staking-reward-curve", @@ -7494,6 +7497,7 @@ dependencies = [ "pallet-balances", "pallet-parameters", "pallet-session", + "pallet-slashing", "pallet-staking", "pallet-staking-reward-curve", "pallet-timestamp", diff --git a/crates/client/entropy_metadata.scale b/crates/client/entropy_metadata.scale index 7ffe949d773fa4b2ff3ef96d9f6c173a033a0360..9d94e92bcafb91bedcad567f69379041060a5b66 100644 GIT binary patch delta 340 zcmYk0%}PQ+7>4Il!AkAlBC;Q%Xw|L@NQ59H8OqAt;pF*_XV7M3W`bOV)H&2Tj0W1Y z6lzf*i~MJa%oGQAj8HrmeI0d?)z)zJZbn&f%iDTo8JfRX9o#C+lG- z;K@H2Aan=&_D+;k~^9BuXFkZ=Ojp0Mh z1Xm7@@RW&7suH zWo29AK1${`$hcVxU=AisK7jemBq?p$tcFgXIOk!xO47u@6)au01L(Gc$lc=) sp_io::TestExternalities { let mut t = system::GenesisConfig::::default().build_storage().unwrap(); diff --git a/pallets/propagation/Cargo.toml b/pallets/propagation/Cargo.toml index e634f2b51..9385b0fbb 100644 --- a/pallets/propagation/Cargo.toml +++ b/pallets/propagation/Cargo.toml @@ -29,10 +29,10 @@ sp-staking ={ version="27.0.0", default-features=false } entropy-shared={ version="0.3.0", path="../../crates/shared", default-features=false, features=[ "wasm-no-std", ] } -pallet-registry={ version="0.3.0", path="../registry", default-features=false } +pallet-attestation={ version="0.3.0", path="../attestation", default-features=false } pallet-programs={ version="0.3.0", path="../programs", default-features=false } +pallet-registry={ version="0.3.0", path="../registry", default-features=false } pallet-staking-extension={ version="0.3.0", path="../staking", default-features=false } -pallet-attestation={ version="0.3.0", path="../attestation", default-features=false } [dev-dependencies] parking_lot="0.12.3" @@ -49,6 +49,7 @@ sp-keystore ={ version="0.35.0" } sp-npos-elections ={ version="27.0.0", default-features=false } pallet-parameters ={ version="0.3.0", path="../parameters", default-features=false } pallet-oracle ={ version='0.3.0', path='../oracle', default-features=false } +pallet-slashing ={ version="0.3.0", path="../slashing", default-features=false } [features] default=['std'] diff --git a/pallets/propagation/src/mock.rs b/pallets/propagation/src/mock.rs index 1fce23e1c..e3d3bf77e 100644 --- a/pallets/propagation/src/mock.rs +++ b/pallets/propagation/src/mock.rs @@ -61,6 +61,7 @@ frame_support::construct_runtime!( Parameters: pallet_parameters, Attestation: pallet_attestation, Oracle: pallet_oracle, + Slashing: pallet_slashing, } ); @@ -394,6 +395,18 @@ impl pallet_attestation::Config for Test { type Randomness = TestPastRandomness; } +parameter_types! { + pub const ReportThreshold: u32 = 5; +} + +impl pallet_slashing::Config for Test { + type RuntimeEvent = RuntimeEvent; + type AuthorityId = UintAuthorityId; + type ReportThreshold = ReportThreshold; + type ValidatorSet = Historical; + type ReportUnresponsiveness = (); +} + // Build genesis storage according to the mock runtime. pub fn new_test_ext() -> sp_io::TestExternalities { let mut t = system::GenesisConfig::::default().build_storage().unwrap(); diff --git a/pallets/registry/Cargo.toml b/pallets/registry/Cargo.toml index 8c092beca..64cbe8362 100644 --- a/pallets/registry/Cargo.toml +++ b/pallets/registry/Cargo.toml @@ -46,6 +46,7 @@ sp-io ={ version="31.0.0", default-features=false } sp-npos-elections ={ version="27.0.0", default-features=false } sp-staking ={ version="27.0.0", default-features=false } pallet-oracle ={ version='0.3.0', path='../oracle', default-features=false } +pallet-slashing ={ version="0.3.0", path="../slashing", default-features=false } [features] diff --git a/pallets/registry/src/benchmarking.rs b/pallets/registry/src/benchmarking.rs index 65849c535..13caabbd4 100644 --- a/pallets/registry/src/benchmarking.rs +++ b/pallets/registry/src/benchmarking.rs @@ -216,7 +216,7 @@ benchmarks! { let network_verifying_key = synedrion::ecdsa::VerifyingKey::try_from(network_verifying_key.as_slice()).unwrap(); - // We substract one from the count since this gets incremented after a succesful registration, + // We subtract one from the count since this gets incremented after a succesful registration, // and we're interested in the account we just registered. let count = >::count() - 1; let derivation_path = diff --git a/pallets/registry/src/mock.rs b/pallets/registry/src/mock.rs index 76054af22..573ea9e75 100644 --- a/pallets/registry/src/mock.rs +++ b/pallets/registry/src/mock.rs @@ -60,6 +60,7 @@ frame_support::construct_runtime!( Programs: pallet_programs, Parameters: pallet_parameters, Oracle: pallet_oracle, + Slashing: pallet_slashing, } ); @@ -380,6 +381,18 @@ impl pallet_parameters::Config for Test { type WeightInfo = (); } +parameter_types! { + pub const ReportThreshold: u32 = 5; +} + +impl pallet_slashing::Config for Test { + type RuntimeEvent = RuntimeEvent; + type AuthorityId = UintAuthorityId; + type ReportThreshold = ReportThreshold; + type ValidatorSet = Historical; + type ReportUnresponsiveness = (); +} + // Build genesis storage according to the mock runtime. pub fn new_test_ext() -> sp_io::TestExternalities { let mut t = system::GenesisConfig::::default().build_storage().unwrap(); diff --git a/pallets/staking/Cargo.toml b/pallets/staking/Cargo.toml index 3cb777887..ff6ecfbbe 100644 --- a/pallets/staking/Cargo.toml +++ b/pallets/staking/Cargo.toml @@ -34,6 +34,7 @@ p256 ={ version="0.13.2", default-features=false, features=["ecdsa" rand ={ version="0.8.5", default-features=false, features=["alloc"] } pallet-parameters={ version="0.3.0", path="../parameters", default-features=false } +pallet-slashing={ version="0.3.0", path="../slashing", default-features=false } entropy-shared={ version="0.3.0", path="../../crates/shared", features=[ "wasm-no-std", ], default-features=false } @@ -69,6 +70,7 @@ std=[ 'pallet-balances/std', 'pallet-parameters/std', 'pallet-session/std', + 'pallet-slashing/std', 'pallet-staking/std', 'scale-info/std', 'sp-consensus-babe/std', diff --git a/pallets/staking/src/benchmarking.rs b/pallets/staking/src/benchmarking.rs index 112a9c978..a6dad4dae 100644 --- a/pallets/staking/src/benchmarking.rs +++ b/pallets/staking/src/benchmarking.rs @@ -29,6 +29,7 @@ use frame_support::{ }; use frame_system::{EventRecord, RawOrigin}; use pallet_parameters::{SignersInfo, SignersSize}; +use pallet_slashing::Event as SlashingEvent; use pallet_staking::{ Event as FrameStakingEvent, MaxNominationsOf, MaxValidatorsCount, Nominations, Pallet as FrameStaking, RewardDestination, ValidatorPrefs, @@ -56,6 +57,16 @@ fn assert_last_event_frame_staking( assert_eq!(event, &system_event); } +fn assert_last_event_slashing( + generic_event: ::RuntimeEvent, +) { + let events = frame_system::Pallet::::events(); + let system_event: ::RuntimeEvent = generic_event.into(); + // compare to the last event record + let EventRecord { event, .. } = &events[events.len() - 1]; + assert_eq!(event, &system_event); +} + pub fn create_validators( count: u32, seed: u32, @@ -518,6 +529,37 @@ benchmarks! { verify { assert!(NextSigners::::get().is_some()); } + + report_unstable_peer { + // We subtract `2` here to give room to our test signers + let s in 0 .. (MAX_SIGNERS - 2) as u32; + + let threshold_reporter: T::AccountId = whitelisted_caller(); + let threshold_offender: T::AccountId = account("threshold_offender", 0, SEED); + + let reporter_validator_id = ::ValidatorId::try_from(threshold_reporter.clone()) + .or(Err(Error::::InvalidValidatorId)) + .unwrap(); + + let offender_validator_id = ::ValidatorId::try_from(threshold_offender.clone()) + .or(Err(Error::::InvalidValidatorId)) + .unwrap(); + + ThresholdToStash::::insert(&threshold_reporter, &reporter_validator_id); + ThresholdToStash::::insert(&threshold_offender, &offender_validator_id); + + let mut signers = vec![reporter_validator_id.clone(); s as usize]; + signers.push(reporter_validator_id); + signers.push(offender_validator_id); + + Signers::::put(signers.clone()); + + }: _(RawOrigin::Signed(threshold_reporter.clone()), threshold_offender.clone()) + verify { + assert_last_event_slashing::( + pallet_slashing::Event::NoteReport(threshold_reporter, threshold_offender).into(), + ); + } } impl_benchmark_test_suite!(Staking, crate::mock::new_test_ext(), crate::mock::Test); diff --git a/pallets/staking/src/lib.rs b/pallets/staking/src/lib.rs index 88bf6cbec..b4961cabe 100644 --- a/pallets/staking/src/lib.rs +++ b/pallets/staking/src/lib.rs @@ -90,6 +90,7 @@ pub mod pallet { + frame_system::Config + pallet_staking::Config + pallet_parameters::Config + + pallet_slashing::Config { /// Because this pallet emits events, it depends on the runtime's definition of an event. type RuntimeEvent: From> + IsType<::RuntimeEvent>; @@ -336,6 +337,7 @@ pub mod pallet { InvalidValidatorId, SigningGroupError, TssAccountAlreadyExists, + NotSigner, NotNextSigner, ReshareNotInProgress, AlreadyConfirmed, @@ -735,6 +737,57 @@ pub mod pallet { // signers see https://github.com/entropyxyz/entropy-core/issues/985 Ok(Pays::No.into()) } + + /// An on-chain hook for TSS servers in the signing committee to report other TSS servers in + /// the committee for misbehaviour. + /// + /// Any "conequences" are handled by the configured Slashing pallet and not this pallet + /// itself. + #[pallet::call_index(7)] + #[pallet::weight(::WeightInfo::report_unstable_peer(MAX_SIGNERS as u32))] + pub fn report_unstable_peer( + origin: OriginFor, + offender_tss_account: T::AccountId, + ) -> DispatchResultWithPostInfo { + let reporter_tss_account = ensure_signed(origin)?; + + // For reporting purposes we need to know the validator account tied to the TSS account. + let reporter_validator_id = Self::threshold_to_stash(&reporter_tss_account) + .ok_or(Error::::NoThresholdKey)?; + let offender_validator_id = Self::threshold_to_stash(&offender_tss_account) + .ok_or(Error::::NoThresholdKey)?; + + // Note: This operation is O(n), but with a small enough Signer group this should be + // fine to do on-chain. + let signers = Self::signers(); + ensure!(signers.contains(&reporter_validator_id), Error::::NotSigner); + ensure!(signers.contains(&offender_validator_id), Error::::NotSigner); + + // We do a bit of a weird conversion here since we want the validator's underlying + // `AccountId` for the reporting mechanism, not their `ValidatorId`. + // + // The Session pallet should have this configured to be the same thing, but we can't + // prove that to the compiler. + let encoded_validator_id = T::ValidatorId::encode(&reporter_validator_id); + let reporter_validator_account = T::AccountId::decode(&mut &encoded_validator_id[..]) + .expect("A `ValidatorId` should be equivalent to an `AccountId`."); + + let encoded_validator_id = T::ValidatorId::encode(&offender_validator_id); + let offending_peer_validator_account = + T::AccountId::decode(&mut &encoded_validator_id[..]) + .expect("A `ValidatorId` should be equivalent to an `AccountId`."); + + // We don't actually take any action here, we offload the reporting to the Slashing + // pallet. + pallet_slashing::Pallet::::note_report( + reporter_validator_account, + offending_peer_validator_account, + )?; + + let actual_weight = + ::WeightInfo::report_unstable_peer(signers.len() as u32); + Ok(Some(actual_weight).into()) + } } impl Pallet { diff --git a/pallets/staking/src/mock.rs b/pallets/staking/src/mock.rs index 21a35e06e..4c71b12b8 100644 --- a/pallets/staking/src/mock.rs +++ b/pallets/staking/src/mock.rs @@ -63,6 +63,7 @@ frame_support::construct_runtime!( Historical: pallet_session_historical, BagsList: pallet_bags_list, Parameters: pallet_parameters, + Slashing: pallet_slashing, } ); @@ -418,6 +419,43 @@ impl entropy_shared::AttestationHandler for MockAttestationHandler { fn request_quote(_attestee: &AccountId, _nonce: [u8; 32]) {} } +type IdentificationTuple = (u64, pallet_staking::Exposure); +type Offence = pallet_slashing::UnresponsivenessOffence; + +parameter_types! { + pub static Offences: Vec = vec![]; +} + +/// A mock offence report handler. +pub struct OffenceHandler; +impl sp_staking::offence::ReportOffence + for OffenceHandler +{ + fn report_offence( + _reporters: Vec, + offence: Offence, + ) -> Result<(), sp_staking::offence::OffenceError> { + Offences::mutate(|l| l.push(offence)); + Ok(()) + } + + fn is_known_offence(_offenders: &[IdentificationTuple], _time_slot: &SessionIndex) -> bool { + false + } +} + +parameter_types! { + pub const ReportThreshold: u32 = 5; +} + +impl pallet_slashing::Config for Test { + type RuntimeEvent = RuntimeEvent; + type AuthorityId = UintAuthorityId; + type ReportThreshold = ReportThreshold; + type ValidatorSet = Historical; + type ReportUnresponsiveness = OffenceHandler; +} + impl pallet_staking_extension::Config for Test { type AttestationHandler = MockAttestationHandler; type Currency = Balances; diff --git a/pallets/staking/src/tests.rs b/pallets/staking/src/tests.rs index 92125b2a9..f2a957b85 100644 --- a/pallets/staking/src/tests.rs +++ b/pallets/staking/src/tests.rs @@ -844,3 +844,58 @@ fn it_stops_chill_when_signer_or_next_signer() { ); }); } + +#[test] +fn cannot_report_outside_of_signer_set() { + new_test_ext().execute_with(|| { + // These mappings come from the mock GenesisConfig + let (alice_validator, alice_tss) = (5, 7); + let (_bob_validator, bob_tss) = (6, 8); + + let (_not_validator, not_tss) = (33, 33); + + // We only want Alice to be part of the signing committee for the test. + Signers::::put(vec![alice_validator]); + + // A TSS which doesn't have a `ValidatorId` cannot report another peer + assert_noop!( + Staking::report_unstable_peer(RuntimeOrigin::signed(not_tss), bob_tss), + Error::::NoThresholdKey + ); + + // A validator which isn't part of the signing committee cannot report another peer + assert_noop!( + Staking::report_unstable_peer(RuntimeOrigin::signed(bob_tss), alice_tss), + Error::::NotSigner + ); + + // An offender that does not have a `ValidatorId` cannot be reported + assert_noop!( + Staking::report_unstable_peer(RuntimeOrigin::signed(alice_tss), not_tss), + Error::::NoThresholdKey + ); + + // An offender which isn't part of the signing committee cannot be reported + assert_noop!( + Staking::report_unstable_peer(RuntimeOrigin::signed(alice_tss), bob_tss), + Error::::NotSigner + ); + }) +} + +#[test] +fn can_report_unstable_peer() { + new_test_ext().execute_with(|| { + // These mappings come from the mock GenesisConfig + let (alice_validator, alice_tss) = (5, 7); + let (bob_validator, bob_tss) = (6, 8); + + Signers::::put(vec![alice_validator, bob_validator]); + + // The TSS accounts are used for reports. We expect the accompanying validator to be + // reported though. + assert_ok!(Staking::report_unstable_peer(RuntimeOrigin::signed(alice_tss), bob_tss)); + + assert_eq!(>::failed_registrations(bob_validator), 1); + }) +} diff --git a/pallets/staking/src/weights.rs b/pallets/staking/src/weights.rs index 012f873ce..e2aa31bfa 100644 --- a/pallets/staking/src/weights.rs +++ b/pallets/staking/src/weights.rs @@ -62,6 +62,7 @@ pub trait WeightInfo { fn confirm_key_reshare_completed() -> Weight; fn new_session_base_weight(s: u32) -> Weight; fn new_session(c: u32, l: u32, v: u32, r: u32) -> Weight; + fn report_unstable_peer(s: u32, ) -> Weight; } /// Weights for pallet_staking_extension using the Substrate node and recommended hardware. @@ -332,6 +333,24 @@ impl WeightInfo for SubstrateWeight { .saturating_add(Weight::from_parts(0, 11364552184692736).saturating_mul(r.into())) .saturating_add(Weight::from_parts(0, 18).saturating_mul(v.into())) } + /// Storage: `StakingExtension::ThresholdToStash` (r:2 w:0) + /// Proof: `StakingExtension::ThresholdToStash` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `StakingExtension::Signers` (r:1 w:0) + /// Proof: `StakingExtension::Signers` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `Slashing::FailedRegistrations` (r:1 w:1) + /// Proof: `Slashing::FailedRegistrations` (`max_values`: None, `max_size`: Some(36), added: 2511, mode: `MaxEncodedLen`) + /// The range of component `s` is `[0, 13]`. + fn report_unstable_peer(s: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `519 + s * (32 ±0)` + // Estimated: `6459 + s * (32 ±0)` + // Minimum execution time: 16_000_000 picoseconds. + Weight::from_parts(19_055_447, 0) + .saturating_add(Weight::from_parts(0, 6459)) + .saturating_add(T::DbWeight::get().reads(4)) + .saturating_add(T::DbWeight::get().writes(1)) + .saturating_add(Weight::from_parts(0, 32).saturating_mul(s.into())) + } } // For backwards compatibility and tests @@ -601,4 +620,22 @@ impl WeightInfo for () { .saturating_add(Weight::from_parts(0, 11364552184692736).saturating_mul(r.into())) .saturating_add(Weight::from_parts(0, 18).saturating_mul(v.into())) } + /// Storage: `StakingExtension::ThresholdToStash` (r:2 w:0) + /// Proof: `StakingExtension::ThresholdToStash` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `StakingExtension::Signers` (r:1 w:0) + /// Proof: `StakingExtension::Signers` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `Slashing::FailedRegistrations` (r:1 w:1) + /// Proof: `Slashing::FailedRegistrations` (`max_values`: None, `max_size`: Some(36), added: 2511, mode: `MaxEncodedLen`) + /// The range of component `s` is `[0, 13]`. + fn report_unstable_peer(s: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `519 + s * (32 ±0)` + // Estimated: `6459 + s * (32 ±0)` + // Minimum execution time: 16_000_000 picoseconds. + Weight::from_parts(19_055_447, 0) + .saturating_add(Weight::from_parts(0, 6459)) + .saturating_add(RocksDbWeight::get().reads(4)) + .saturating_add(RocksDbWeight::get().writes(1)) + .saturating_add(Weight::from_parts(0, 32).saturating_mul(s.into())) + } } diff --git a/runtime/src/weights/pallet_staking_extension.rs b/runtime/src/weights/pallet_staking_extension.rs index a6bc32bad..0132f9f95 100644 --- a/runtime/src/weights/pallet_staking_extension.rs +++ b/runtime/src/weights/pallet_staking_extension.rs @@ -321,4 +321,22 @@ impl pallet_staking_extension::WeightInfo for WeightInf .saturating_add(Weight::from_parts(0, 11364552184692736).saturating_mul(r.into())) .saturating_add(Weight::from_parts(0, 18).saturating_mul(v.into())) } + /// Storage: `StakingExtension::ThresholdToStash` (r:2 w:0) + /// Proof: `StakingExtension::ThresholdToStash` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `StakingExtension::Signers` (r:1 w:0) + /// Proof: `StakingExtension::Signers` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `Slashing::FailedRegistrations` (r:1 w:1) + /// Proof: `Slashing::FailedRegistrations` (`max_values`: None, `max_size`: Some(36), added: 2511, mode: `MaxEncodedLen`) + /// The range of component `s` is `[0, 13]`. + fn report_unstable_peer(s: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `519 + s * (32 ±0)` + // Estimated: `6459 + s * (32 ±0)` + // Minimum execution time: 16_000_000 picoseconds. + Weight::from_parts(19_055_447, 0) + .saturating_add(Weight::from_parts(0, 6459)) + .saturating_add(T::DbWeight::get().reads(4)) + .saturating_add(T::DbWeight::get().writes(1)) + .saturating_add(Weight::from_parts(0, 32).saturating_mul(s.into())) + } } From 6fa0befd4f12747552e01761fece7c1a9a65d6f7 Mon Sep 17 00:00:00 2001 From: peg Date: Wed, 18 Dec 2024 08:20:30 +0100 Subject: [PATCH 2/2] Generate mnemonic internally in `entropy-tss` (#1128) * Dont allow mnemonic to be passed in via CLI, or environment variable - generate it internally * Changelog * Error handling * Add endpoint giving public keys * Document new endpoint * Changelog * Clippy * Fix lockfile * Fix changelog following merge master * Changelog changes following review --- CHANGELOG.md | 5 +++ .../src/helpers/launch.rs | 31 ------------- crates/threshold-signature-server/src/lib.rs | 8 +++- crates/threshold-signature-server/src/main.rs | 43 ++++--------------- .../src/node_info/api.rs | 26 ++++++++++- .../src/node_info/errors.rs | 34 +++++++++++++++ .../src/node_info/mod.rs | 1 + .../src/node_info/tests.rs | 25 ++++++++++- 8 files changed, 104 insertions(+), 69 deletions(-) create mode 100644 crates/threshold-signature-server/src/node_info/errors.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index efa13cc42..4f17c8377 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,9 @@ At the moment this project **does not** adhere to - In [#1153](https://github.com/entropyxyz/entropy-core/pull/1153/) the program runtime was updated to accept multiple oracle inputs, this means any programs that were compiled and used need to be recompiled to the new runtime +- In [#1128](https://github.com/entropyxyz/entropy-core/pull/1128) mnemonics can no longer be passed + in to `entropy-tss` via the `--mnemonic` command line argument, a file, or an environment variable. + Instead they are randomly generated internally and can be retrieved with the `/info` HTTP route. - In [#1179](https://github.com/entropyxyz/entropy-core/pull/1179) the format of TDX Quote input data has been changed. - In [#1147](https://github.com/entropyxyz/entropy-core/pull/1147) a field is added to the @@ -31,6 +34,8 @@ runtime for testing. If this is not desired it should be set to `None`. ### Added +- In [#1128](https://github.com/entropyxyz/entropy-core/pull/1128) an `/info` route was added to `entropy-tss` + which can be used to get the TSS account ID and x25519 public key. - Protocol message versioning ([#1140](https://github.com/entropyxyz/entropy-core/pull/1140)) - CLI command to get oracle headings ([#1170](https://github.com/entropyxyz/entropy-core/pull/1170)) - Add TSS endpoint to get TDX quote ([#1173](https://github.com/entropyxyz/entropy-core/pull/1173)) diff --git a/crates/threshold-signature-server/src/helpers/launch.rs b/crates/threshold-signature-server/src/helpers/launch.rs index 444d98f62..260fbdb47 100644 --- a/crates/threshold-signature-server/src/helpers/launch.rs +++ b/crates/threshold-signature-server/src/helpers/launch.rs @@ -207,37 +207,6 @@ pub struct StartupArgs { /// Returns the AccountID and Diffie-Hellman Public Keys associated with this server. #[arg(long = "setup-only")] pub setup_only: bool, - - /// The BIP-39 mnemonic (i.e seed phrase) to use for deriving the Threshold Signature Server - /// SR25519 account ID and the X25519 public key. - /// - /// The SR25519 account is responsible for signing and submitting extrinsics to the Entropy - /// network. - /// - /// The X25519 public key is used for encrypting/decrypting messages to other threshold - /// servers. - /// - /// Note that this may keep a mnemonic in your shell history. If you would like to avoid this - /// use one of the alternative options, or tools like the 1Password CLI. - /// - /// **Alternatives**: There are two other ways to supply the mnemonic to the TSS: the - /// `--mnemonic-file` flag and the `THRESHOLD_SERVER_MNEMONIC` environment variable. - /// - /// **Warning**: Passing this flag will overwrite any existing mnemonic! If you would like to - /// use an existing mnemonic omit this flag when running the process. - #[arg(long = "mnemonic")] - pub mnemonic: Option, - - /// The path to a file containing the BIP-39 mnemonic (i.e seed phrase) to use for deriving the - /// Threshold Signature Server SR25519 account ID and the X25519 public key. - /// - /// **Alternatives**: There are two other ways to supply the mnemonic to the TSS: the - /// `--mnemonic` flag and the `THRESHOLD_SERVER_MNEMONIC` environment variable. - /// - /// **Warning**: Passing this flag will overwrite any existing mnemonic! If you would like to - /// use an existing mnemonic omit this flag when running the process. - #[arg(long = "mnemonic-file", conflicts_with = "mnemonic")] - pub mnemonic_file: Option, } pub async fn has_mnemonic(kv: &KvManager) -> bool { diff --git a/crates/threshold-signature-server/src/lib.rs b/crates/threshold-signature-server/src/lib.rs index 2468208b2..5cd01cb49 100644 --- a/crates/threshold-signature-server/src/lib.rs +++ b/crates/threshold-signature-server/src/lib.rs @@ -110,6 +110,11 @@ //! http://127.0.0.1:3001/user/sign_tx //! ``` //! +//! ### For the node operator +//! +//! [`/info`](crate::node_info::api::info()) - Get - get a Json object of type +//! [crate::node_info::api::TssPublicKeys] which contains the TSS account ID and x25519 public key. +//! //! ### For the blockchain node //! //! ### For other instances of the threshold server @@ -185,7 +190,7 @@ use crate::{ attestation::api::{attest, get_attest}, health::api::healthz, launch::Configuration, - node_info::api::{hashes, version as get_version}, + node_info::api::{hashes, info, version as get_version}, r#unsafe::api::{delete, put, remove_keys, unsafe_get}, signing_client::{api::*, ListenerState}, user::api::*, @@ -218,6 +223,7 @@ pub fn app(app_state: AppState) -> Router { .route("/healthz", get(healthz)) .route("/version", get(get_version)) .route("/hashes", get(hashes)) + .route("/info", get(info)) .route("/ws", get(ws_handler)); // Unsafe routes are for testing purposes only diff --git a/crates/threshold-signature-server/src/main.rs b/crates/threshold-signature-server/src/main.rs index c9e520e7d..ec00329b0 100644 --- a/crates/threshold-signature-server/src/main.rs +++ b/crates/threshold-signature-server/src/main.rs @@ -20,8 +20,8 @@ use clap::Parser; use entropy_tss::{ app, launch::{ - development_mnemonic, load_kv_store, setup_latest_block_number, setup_mnemonic, setup_only, - Configuration, StartupArgs, ValidatorName, + development_mnemonic, has_mnemonic, load_kv_store, setup_latest_block_number, + setup_mnemonic, setup_only, Configuration, StartupArgs, ValidatorName, }, AppState, }; @@ -66,39 +66,14 @@ async fn main() { let app_state = AppState::new(configuration.clone(), kv_store.clone()); - // We consider the inputs in order of most to least explicit: CLI flag, supplied file, - // environment variable. - let user_mnemonic = args - .mnemonic - .or_else(|| { - args.mnemonic_file.map(|path| { - let file = std::fs::read(path).expect("Unable to read mnemonic file."); - let mnemonic = std::str::from_utf8(&file) - .expect("Unable to convert provided mnemonic to UTF-8 string.") - .trim(); - - bip39::Mnemonic::parse_normalized(mnemonic) - .expect("Unable to parse given mnemonic.") - }) - }) - .or_else(|| { - std::env::var("THRESHOLD_SERVER_MNEMONIC").ok().map(|mnemonic| { - bip39::Mnemonic::parse_normalized(&mnemonic) - .expect("Unable to parse given mnemonic.") - }) - }); - - if let Some(mnemonic) = user_mnemonic { - setup_mnemonic(&kv_store, mnemonic).await - } else if cfg!(test) || validator_name.is_some() { + if cfg!(test) || validator_name.is_some() { setup_mnemonic(&kv_store, development_mnemonic(&validator_name)).await - } else { - let has_mnemonic = entropy_tss::launch::has_mnemonic(&kv_store).await; - assert!( - has_mnemonic, - "No mnemonic provided. Please provide one or use a development account." - ); - }; + } else if !has_mnemonic(&kv_store).await { + let mut rng = rand::thread_rng(); + let mnemonic = bip39::Mnemonic::generate_in_with(&mut rng, bip39::Language::English, 24) + .expect("Failed to generate mnemonic"); + setup_mnemonic(&kv_store, mnemonic).await + } setup_latest_block_number(&kv_store).await.expect("Issue setting up Latest Block Number"); diff --git a/crates/threshold-signature-server/src/node_info/api.rs b/crates/threshold-signature-server/src/node_info/api.rs index 23a0e1526..52aa40f73 100644 --- a/crates/threshold-signature-server/src/node_info/api.rs +++ b/crates/threshold-signature-server/src/node_info/api.rs @@ -12,9 +12,13 @@ // // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -use axum::Json; -use entropy_shared::types::HashingAlgorithm; +use crate::{get_signer_and_x25519_secret, node_info::errors::GetInfoError, AppState}; +use axum::{extract::State, Json}; +use entropy_shared::{types::HashingAlgorithm, X25519PublicKey}; +use serde::{Deserialize, Serialize}; +use sp_core::Pair; use strum::IntoEnumIterator; +use subxt::utils::AccountId32; /// Returns the version and commit data #[tracing::instrument] @@ -22,8 +26,26 @@ pub async fn version() -> String { format!("{}-{}", env!("CARGO_PKG_VERSION"), env!("VERGEN_GIT_DESCRIBE")) } +/// Lists the supported hashing algorithms #[tracing::instrument] pub async fn hashes() -> Json> { let hashing_algos = HashingAlgorithm::iter().collect::>(); Json(hashing_algos) } + +/// Public signing and encryption keys associated with a TS server +#[derive(Serialize, Deserialize, Clone, Debug, Eq, PartialEq)] +pub struct TssPublicKeys { + pub tss_account: AccountId32, + pub x25519_public_key: X25519PublicKey, +} + +/// Returns the TS server's public keys and HTTP endpoint +#[tracing::instrument(skip_all)] +pub async fn info(State(app_state): State) -> Result, GetInfoError> { + let (signer, x25519_secret) = get_signer_and_x25519_secret(&app_state.kv_store).await?; + let tss_account = AccountId32(signer.signer().public().0); + let x25519_public_key = *x25519_dalek::PublicKey::from(&x25519_secret).as_bytes(); + + Ok(Json(TssPublicKeys { x25519_public_key, tss_account })) +} diff --git a/crates/threshold-signature-server/src/node_info/errors.rs b/crates/threshold-signature-server/src/node_info/errors.rs new file mode 100644 index 000000000..9936634f9 --- /dev/null +++ b/crates/threshold-signature-server/src/node_info/errors.rs @@ -0,0 +1,34 @@ +// Copyright (C) 2023 Entropy Cryptography Inc. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . +use axum::{ + http::StatusCode, + response::{IntoResponse, Response}, +}; +use thiserror::Error; + +/// Errors for protocol execution +#[derive(Debug, Error)] +pub enum GetInfoError { + #[error("Could not get public keys: {0}")] + User(#[from] crate::user::errors::UserErr), +} + +impl IntoResponse for GetInfoError { + fn into_response(self) -> Response { + tracing::error!("{:?}", format!("{self}")); + let body = format!("{self}").into_bytes(); + (StatusCode::INTERNAL_SERVER_ERROR, body).into_response() + } +} diff --git a/crates/threshold-signature-server/src/node_info/mod.rs b/crates/threshold-signature-server/src/node_info/mod.rs index 5837e4d77..3dcdf1f61 100644 --- a/crates/threshold-signature-server/src/node_info/mod.rs +++ b/crates/threshold-signature-server/src/node_info/mod.rs @@ -15,6 +15,7 @@ //! Provides information about this instance of `entropy-tss` pub mod api; +mod errors; #[cfg(test)] mod tests; diff --git a/crates/threshold-signature-server/src/node_info/tests.rs b/crates/threshold-signature-server/src/node_info/tests.rs index ba851a702..2676ecb87 100644 --- a/crates/threshold-signature-server/src/node_info/tests.rs +++ b/crates/threshold-signature-server/src/node_info/tests.rs @@ -13,9 +13,13 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -use crate::helpers::tests::{initialize_test_logger, setup_client}; +use crate::{ + helpers::tests::{initialize_test_logger, setup_client}, + node_info::api::TssPublicKeys, +}; use entropy_kvdb::clean_tests; use entropy_shared::types::HashingAlgorithm; +use entropy_testing_utils::constants::{TSS_ACCOUNTS, X25519_PUBLIC_KEYS}; use serial_test::serial; #[tokio::test] @@ -55,3 +59,22 @@ async fn hashes_test() { ); clean_tests(); } + +#[tokio::test] +#[serial] +async fn info_test() { + clean_tests(); + initialize_test_logger().await; + setup_client().await; + let client = reqwest::Client::new(); + let response = client.get("http://127.0.0.1:3001/info").send().await.unwrap(); + let public_keys: TssPublicKeys = response.json().await.unwrap(); + assert_eq!( + public_keys, + TssPublicKeys { + tss_account: TSS_ACCOUNTS[0].clone(), + x25519_public_key: X25519_PUBLIC_KEYS[0], + } + ); + clean_tests(); +}