Skip to content

Commit

Permalink
Change attestation flow to be pull based (#1109)
Browse files Browse the repository at this point in the history
* Add trait for verifying attestations

* Clean up some of the Staking Config comments

* Use `AttestationHandler` in Staking pallet

* Add `Attestation::request_attestation()` extrinsic

* Check that there's only one attestation request per account

* Write to `Threshold*` data structures in `validate()`

Since we're verifying the attestation in the extrinsic we can also write to these structures in the
extrinsic again.

* Allow mock attestation handler to fail

* Add convenience implementation for `AttestationHandler

* Do some cleanup in Attestation pallet

* For `validate` tests don't use the `ValidationQueue`

* Input expected keys into quote verification method directly

We need to match what the TSS is committing to (e.g what we write to storage) on the Staking pallet
end, but we don't always have that information available on the Attestation pallet side. By having
the expected data come in alongside the quote we can more easily check that it does match.

* Add test for checking failed quote verification

* Add some docs explaining how the quote request system works

* Remove `AttestationQueue`, `KeyProvider`, and `ValidationQueue`

We don't need all this infrastructure anymore since requests managed better by individual pallets.

* Clean up some of the staking tests

* Add more relevant event after `validate()` is successful

* Update runtime implementation

* Bump metadata

* Ignore the TSS attestation tests

Not sure if we're gonna need this in their current form anymore. Need to discuss with the others.

* Feature gate `AttestationHandler`

We don't want it to get used for the Client's `wasm` builds.

* Missed a few `AttestationHandler` mock implementations

* Remove unused associated types

* Get rid of `on_initialize()` benchmarks

* Remove `MaxPendingAttestations` associated type

* Add method for requesting quotes to `AttestationHandler`

This let's us request quotes in a non-extrinsic way, useful for testing and benchmarking.

* Use threshold account when verifying quote

This was using the caller, who is a Substrate/Validator account.

* Get `validate()` bench working again

We're now using "real" PCKs to get this to pass

* Add `nonce` to `request_quote()`

This will make it easier for us to control the expected nonce in the context of benchmarking and
testing.

* Accept more quote types in Staking mock

* Tidy up `validate()` bench

* Strip down bench for `attest` and add one for `request_attestation`

* Use better dummy endpoint in `validate()` bench

* RustFmt

* Remove Attestation pallet from Staking mock

* Remove TODO

* Add `CHANGELOG` entry

* Remove another TODO, this should become an issue

* TaploFmt

* Use published version of `tdx-quote`

* Mention one more config type added in `CHANGELOG`
  • Loading branch information
HCastano authored Oct 18, 2024
1 parent 18ee31f commit 13910ee
Show file tree
Hide file tree
Showing 21 changed files with 433 additions and 734 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,14 @@ At the moment this project **does not** adhere to

### Breaking Changes
- In [#1104](https://github.com/entropyxyz/entropy-core/pull/1104) the `/validator/rotate_network_key` endpoint was renamed to `rotate_network_key`
- In [#1109](https://github.com/entropyxyz/entropy-core/pull/1109/), the `MaxPendingAttestations` config type, the `ValidationQueue` storage
structure, and the `NodeInfoChanged` event were removed from the Staking Extension pallet. The
`AttestationHandler` config type was added to the Staking Extension pallet. The `KeyProvider` and
`AttestationQueue` config types were removed from the Attestation pallet.

### Changed
- Use correct key rotation endpoint in OCW ([#1104](https://github.com/entropyxyz/entropy-core/pull/1104))
- Change attestation flow to be pull based ([#1109](https://github.com/entropyxyz/entropy-core/pull/1109/))

## [0.3.0-rc.1](https://github.com/entropyxyz/entropy-core/compare/release/v0.2.0...release/v0.3.0-rc.1) - 2024-10-04

Expand Down
19 changes: 4 additions & 15 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file modified crates/client/entropy_metadata.scale
Binary file not shown.
54 changes: 28 additions & 26 deletions crates/shared/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,33 +134,35 @@ impl QuoteInputData {
}
}

/// A trait used to get different stored keys for a given account ID.
///
/// Not every account ID will have an given key, in which case the implementer is expected to
/// return `None`.
pub trait KeyProvider<T> {
/// Get an X25519 public key, if any, for the given account ID.
fn x25519_public_key(account_id: &T) -> Option<X25519PublicKey>;

/// Get a provisioning certification key, if any, for the given account ID.
fn provisioning_key(account_id: &T) -> Option<EncodedVerifyingKey>;
/// A trait for types which can handle attestation requests.
#[cfg(not(feature = "wasm"))]
pub trait AttestationHandler<AccountId> {
/// Verify that the given quote is valid and matches the given information about the attestee.
fn verify_quote(
attestee: &AccountId,
x25519_public_key: X25519PublicKey,
provisioning_certification_key: BoundedVecEncodedVerifyingKey,
quote: Vec<u8>,
) -> Result<(), sp_runtime::DispatchError>;

/// Indicate to the attestation handler that a quote is desired.
///
/// The `nonce` should be a piece of data (e.g a random number) which indicates that the quote
/// is reasonably fresh and has not been reused.
fn request_quote(attestee: &AccountId, nonce: [u8; 32]);
}

/// A trait used to describe a queue of attestations.
pub trait AttestationQueue<T> {
/// Indicate that a given attestation is ready to be moved from a pending state to a confirmed
/// state.
fn confirm_attestation(account_id: &T);

/// Request that an attestation get added to the queue for later processing.
fn push_pending_attestation(
signer: T,
tss_account: T,
x25519_public_key: X25519PublicKey,
endpoint: Vec<u8>,
provisioning_certification_key: EncodedVerifyingKey,
);
/// A convenience implementation for testing and benchmarking.
#[cfg(not(feature = "wasm"))]
impl<AccountId> AttestationHandler<AccountId> for () {
fn verify_quote(
_attestee: &AccountId,
_x25519_public_key: X25519PublicKey,
_provisioning_certification_key: BoundedVecEncodedVerifyingKey,
_quote: Vec<u8>,
) -> Result<(), sp_runtime::DispatchError> {
Ok(())
}

/// The list of pending (not processed) attestations.
fn pending_attestations() -> Vec<T>;
fn request_quote(_attestee: &AccountId, _nonce: [u8; 32]) {}
}
2 changes: 2 additions & 0 deletions crates/threshold-signature-server/src/attestation/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use entropy_testing_utils::{
};
use serial_test::serial;

#[ignore]
#[tokio::test]
#[serial]
async fn test_attest() {
Expand Down Expand Up @@ -72,6 +73,7 @@ async fn test_attest() {
panic!("Waited 10 blocks and attestation is still pending");
}

#[ignore]
#[tokio::test]
#[serial]
async fn test_attest_validation_fail() {
Expand Down
85 changes: 9 additions & 76 deletions pallets/attestation/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,13 @@
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use entropy_shared::{AttestationQueue, QuoteInputData};
use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller};
use frame_benchmarking::{benchmarks, impl_benchmark_test_suite, whitelisted_caller};
use frame_system::{EventRecord, RawOrigin};

use super::*;
#[allow(unused)]
use crate::Pallet as AttestationPallet;

/// This is a randomly generated secret p256 ECDSA key - for mocking attestation
const ATTESTATION_KEY: [u8; 32] = [
167, 184, 203, 130, 240, 249, 191, 129, 206, 9, 200, 29, 99, 197, 64, 81, 135, 166, 59, 73, 31,
27, 206, 207, 69, 248, 56, 195, 64, 92, 109, 46,
];

/// This is a randomly generated secret p256 ECDSA key - for mocking the provisioning certification
/// key
const PCK: [u8; 32] = [
117, 153, 212, 7, 220, 16, 181, 32, 110, 138, 4, 68, 208, 37, 104, 54, 1, 110, 232, 207, 100,
168, 16, 99, 66, 83, 21, 178, 81, 155, 132, 37,
];

fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
let events = frame_system::Pallet::<T>::events();
let system_event: <T as frame_system::Config>::RuntimeEvent = generic_event.into();
Expand All @@ -45,76 +31,23 @@ fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
benchmarks! {
attest {
let attestee: T::AccountId = whitelisted_caller();
let nonce = [0; 32];

let attestation_key = tdx_quote::SigningKey::from_bytes(&ATTESTATION_KEY.into()).unwrap();
let pck = tdx_quote::SigningKey::from_bytes(&PCK.into()).unwrap();
let pck_encoded = tdx_quote::encode_verifying_key(pck.verifying_key()).unwrap();

let input_data = QuoteInputData::new(
&attestee, // TSS Account ID
[0; 32], // x25519 public key
nonce,
1, // Block number
);
let quote = tdx_quote::Quote::mock(attestation_key.clone(), pck, input_data.0).as_bytes().to_vec();

// Insert a pending attestation so that this quote is expected
<PendingAttestations<T>>::insert(attestee.clone(), nonce);

// We also need to write to the queue (whose specific implementation writes to the staking
// pallet in this case) to ensure that the `attest` extrinsic has all the information about our
// attestee available.
T::AttestationQueue::push_pending_attestation(
attestee.clone(),
attestee.clone(),
[0; 32],
b"http://localhost:3001".to_vec(),
pck_encoded,
);
let quote = [0; 32].to_vec();
}: _(RawOrigin::Signed(attestee.clone()), quote.clone())
verify {
assert_last_event::<T>(
Event::<T>::AttestationMade.into()
);

// Check that there is no longer a pending attestation
assert!(!<PendingAttestations<T>>::contains_key(attestee));
}

on_initialize {
// Note: We should technically be using `MaxPendingAttestations` but we don't have access to
// that here...
let s in 1 .. 250;

let caller: T::AccountId = whitelisted_caller();
let nonce = [0; 32];
let block_number = <frame_system::Pallet<T>>::block_number();

let pck = tdx_quote::SigningKey::from_bytes(&PCK.into()).unwrap();
let pck_encoded = tdx_quote::encode_verifying_key(pck.verifying_key()).unwrap();

for i in 0..s {
let threshold_account_id: T::AccountId = account("threshold", 0, i);
T::AttestationQueue::push_pending_attestation(
threshold_account_id.clone(),
threshold_account_id.clone(),
[0; 32],
b"http://localhost:3001".to_vec(),
pck_encoded,
);
}
}: {
use frame_support::traits::Hooks;
let _ = AttestationPallet::<T>::on_initialize(block_number);
} verify {
// Here we'll just spot check one account instead of `s` accounts since if one was written
// all were
let threshold_account_id: T::AccountId = account("threshold", 0, 0);
assert!(PendingAttestations::<T>::get(threshold_account_id).is_some());

// We want to ensure that all the requests that we added to the `T::AttestationQueue` were
// also added to `AttestationRequests`
assert!(AttestationRequests::<T>::get(block_number).unwrap().len() == s as usize);
request_attestation {
let attestee: T::AccountId = whitelisted_caller();
}: _(RawOrigin::Signed(attestee.clone()))
verify {
// We're expecting a pending attestation queued up
assert!(<PendingAttestations<T>>::contains_key(attestee));
}
}

Expand Down
Loading

0 comments on commit 13910ee

Please sign in to comment.