Skip to content

Commit

Permalink
Signing flow with derived accounts (#990)
Browse files Browse the repository at this point in the history
* Store `derivation_path` in `RegisteredInfo`

* Bump metadata

* Split registration flow based on existence of derivation path

* Tiny up a few things related to jumpstart mocking

* Small grammatical tidyings

* Remove extra function introduced in merge

* Fix a few typos

* Add a few logging events

* Allow querying both new and old signers using helper

* Add working test for signature request with derived account

* Fix Registry test compilation

* Don't ignore old signing test

* Fix Registry benchmark compilation

* Handle BIP-32 errors instead of panicking

* Store full BIP-32 derivation path on-chain

* Add `CHANGELOG` entry

* Sort signers from Staking Extension pallet when signing

* Revert "Sort signers from Staking Extension pallet when signing"

Turns out this breaks the signing flow for some reason 🤷
  • Loading branch information
HCastano authored Aug 14, 2024
1 parent 314f6be commit 13a365f
Show file tree
Hide file tree
Showing 18 changed files with 271 additions and 49 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ At the moment this project **does not** adhere to
- Reshare confirmation ([#965](https://github.com/entropyxyz/entropy-core/pull/965))
- Set inital signers ([#971](https://github.com/entropyxyz/entropy-core/pull/971))
- Add parent key threshold dynamically ([#974](https://github.com/entropyxyz/entropy-core/pull/974))
- Signing flow with derived accounts ([#990](https://github.com/entropyxyz/entropy-core/pull/990))
- TSS attestation endpoint ([#1001](https://github.com/entropyxyz/entropy-core/pull/1001))
- Add `network-jumpstart` command to `entropy-test-cli` ([#1004](https://github.com/entropyxyz/entropy-core/pull/1004))

Expand Down
3 changes: 3 additions & 0 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.
6 changes: 3 additions & 3 deletions crates/client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ pub async fn sign(
auxilary_data: Option<Vec<u8>>,
) -> Result<RecoverableSignature, ClientError> {
let message_hash = Hasher::keccak(&message);
let validators_info = get_signers_from_chain(api, rpc).await?;
let validators_info = get_signers_from_chain(api, rpc, false).await?;
tracing::debug!("Validators info {:?}", validators_info);
let block_number = rpc.chain_get_header(None).await?.ok_or(ClientError::BlockNumber)?.number;
let signature_request = UserSignatureRequest {
Expand Down Expand Up @@ -295,9 +295,9 @@ pub async fn put_register_request_on_chain(
rpc: &LegacyRpcMethods<EntropyConfig>,
signature_request_keypair: sr25519::Pair,
deployer: SubxtAccountId32,
program_instance: BoundedVec<ProgramInstance>,
program_instances: BoundedVec<ProgramInstance>,
) -> Result<(), ClientError> {
let registering_tx = entropy::tx().registry().register(deployer, program_instance);
let registering_tx = entropy::tx().registry().register(deployer, program_instances);

submit_transaction_with_pair(api, rpc, &signature_request_keypair, &registering_tx, None)
.await?;
Expand Down
30 changes: 22 additions & 8 deletions crates/client/src/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,27 +36,39 @@ pub struct UserSignatureRequest {
pub block_number: BlockNumber,
/// Hashing algorithm to be used for signing
pub hash: HashingAlgorithm,
/// The veryfying key for the signature requested
/// The verifying key for the signature requested
pub signature_verifying_key: Vec<u8>,
}

pub async fn get_signers_from_chain(
api: &OnlineClient<EntropyConfig>,
rpc: &LegacyRpcMethods<EntropyConfig>,
with_parent_key: bool,
) -> Result<Vec<ValidatorInfo>, SubgroupGetError> {
let all_validators_query = entropy::storage().session().validators();
let mut validators = query_chain(api, rpc, all_validators_query, None)
.await?
.ok_or_else(|| SubgroupGetError::ChainFetch("Get all validators error"))?;
let block_hash = rpc.chain_get_block_hash(None).await?;
let mut handles = Vec::new();
let mut validators = if with_parent_key {
let signer_query = entropy::storage().staking_extension().signers();
query_chain(api, rpc, signer_query, None)
.await?
.ok_or_else(|| SubgroupGetError::ChainFetch("Get all validators error"))?
} else {
let all_validators_query = entropy::storage().session().validators();
let mut validators = query_chain(api, rpc, all_validators_query, None)
.await?
.ok_or_else(|| SubgroupGetError::ChainFetch("Get all validators error"))?;

validators.sort();
validators
};

// TODO #898 For now we use a fix proportion of the number of validators as the threshold
let threshold = (validators.len() as f32 * 0.75) as usize;

// TODO #899 For now we just take the first t validators as the ones to perform signing
validators.sort();
validators.truncate(threshold);

let block_hash = rpc.chain_get_block_hash(None).await?;
let mut handles = Vec::new();

for validator in validators {
let handle: tokio::task::JoinHandle<Result<ValidatorInfo, SubgroupGetError>> =
tokio::task::spawn({
Expand All @@ -77,8 +89,10 @@ pub async fn get_signers_from_chain(
})
}
});

handles.push(handle);
}

let mut all_signers: Vec<ValidatorInfo> = vec![];
for handle in handles {
all_signers.push(handle.await??);
Expand Down
3 changes: 2 additions & 1 deletion crates/threshold-signature-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@ uuid ={ version="1.10.0", features=["v4"] }

# Misc
tokio-tungstenite="0.23.1"
bip39 ={ version="2.0.0", features=["zeroize"] }
bincode ="1.3.3"
bip32 ={ version="0.5.2" }
bip39 ={ version="2.0.0", features=["zeroize"] }
bytes ={ version="1.7", default-features=false, features=["serde"] }
base64 ="0.22.1"
clap ={ version="4.5.15", features=["derive"] }
Expand Down
6 changes: 4 additions & 2 deletions crates/threshold-signature-server/src/helpers/signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ pub async fn do_signing(
app_state: &AppState,
signing_session_info: SigningSessionInfo,
request_limit: u32,
derivation_path: Option<bip32::DerivationPath>,
) -> Result<RecoverableSignature, ProtocolErr> {
tracing::debug!("Preparing to perform signing");

Expand All @@ -60,8 +61,8 @@ pub async fn do_signing(

let account_id = AccountId32(signer.public().0);

// set up context for signing protocol execution
let sign_context = signing_service.get_sign_context(info.clone()).await?;
// Set up context for signing protocol execution
let sign_context = signing_service.get_sign_context(info.clone(), derivation_path).await?;

let tss_accounts: Vec<AccountId32> = user_signature_request
.validators_info
Expand Down Expand Up @@ -89,6 +90,7 @@ pub async fn do_signing(
&x25519_secret_key,
)
.await?;

let channels = {
let ready = timeout(Duration::from_secs(SETUP_TIMEOUT_SECONDS), rx_ready).await?;
let broadcast_out = ready??;
Expand Down
4 changes: 3 additions & 1 deletion crates/threshold-signature-server/src/helpers/substrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,12 @@ pub async fn get_registered_details(
let registered_result = query_chain(api, rpc, registered_info_query, None).await?;

let registration_info = if let Some(old_registration_info) = registered_result {
tracing::debug!("Found user in old `Registered` struct.");

old_registration_info
} else {
// We failed with the old registration path, let's try the new one
tracing::warn!("Didn't find user in old `Registered` struct, trying new one");
tracing::warn!("Didn't find user in old `Registered` struct, trying new one.");

let registered_info_query =
entropy::storage().registry().registered_on_chain(BoundedVec(verifying_key));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ pub enum ProtocolErr {
SubstrateClient(#[from] entropy_client::substrate::SubstrateError),
#[error("Listener: {0}")]
Listener(#[from] entropy_protocol::errors::ListenerErr),
#[error("Failed to derive BIP-32 account: {0}")]
Bip32DerivationError(#[from] bip32::Error),
}

impl IntoResponse for ProtocolErr {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,34 @@ impl<'a> ThresholdSigningService<'a> {
fields(sign_init),
level = tracing::Level::DEBUG
)]
pub async fn get_sign_context(&self, sign_init: SignInit) -> Result<SignContext, ProtocolErr> {
pub async fn get_sign_context(
&self,
sign_init: SignInit,
derivation_path: Option<bip32::DerivationPath>,
) -> Result<SignContext, ProtocolErr> {
tracing::debug!("Getting signing context");
let key_share_and_aux_info_vec = self
.kv_manager
.kv()
.get(&hex::encode(sign_init.signing_session_info.signature_verifying_key.clone()))
.await?;

let verifying_key = if derivation_path.is_some() {
entropy_shared::NETWORK_PARENT_KEY.as_bytes().to_vec()
} else {
sign_init.signing_session_info.signature_verifying_key.clone()
};

let key_share_and_aux_info_vec =
self.kv_manager.kv().get(&hex::encode(verifying_key)).await?;

let (key_share, aux_info): (
ThresholdKeyShare<KeyParams, PartyId>,
AuxInfo<KeyParams, PartyId>,
) = entropy_kvdb::kv_manager::helpers::deserialize(&key_share_and_aux_info_vec)
.ok_or_else(|| ProtocolErr::Deserialization("Failed to load KeyShare".into()))?;

let key_share = if let Some(path) = derivation_path {
key_share.derive_bip32(&path)?
} else {
key_share
};

Ok(SignContext::new(sign_init, key_share, aux_info))
}

Expand All @@ -92,6 +108,7 @@ impl<'a> ThresholdSigningService<'a> {
threshold_signer: &sr25519::Pair,
threshold_accounts: Vec<AccountId32>,
) -> Result<RecoverableSignature, ProtocolErr> {
tracing::debug!("Executing signing session");
tracing::trace!("Signing info {session_id:?}");

let message_hash = if let SessionId::Sign(session_info) = &session_id {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ pub async fn open_protocol_connections(
state: &ListenerState,
x25519_secret_key: &x25519_dalek::StaticSecret,
) -> Result<(), ProtocolErr> {
tracing::debug!("Opening protocol connections");

let connect_to_validators = validators_info
.iter()
.filter(|validators_info| {
Expand Down
51 changes: 38 additions & 13 deletions crates/threshold-signature-server/src/user/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ pub async fn sign_tx(
.number;

check_stale(user_sig_req.block_number, block_number).await?;

// Probably impossible but block signing from parent key anyways
if string_verifying_key == hex::encode(NETWORK_PARENT_KEY) {
return Err(UserErr::NoSigningFromParentKey);
Expand All @@ -159,7 +160,9 @@ pub async fn sign_tx(
if user_details.programs_data.0.is_empty() {
return Err(UserErr::NoProgramPointerDefined());
}
// handle aux data padding, if it is not explicit by client for ease send through None, error if incorrect length

// Handle aux data padding, if it is not explicit by client for ease send through None, error
// if incorrect length
let auxilary_data_vec;
if let Some(auxilary_data) = user_sig_req.clone().auxilary_data {
if auxilary_data.len() < user_details.programs_data.0.len() {
Expand All @@ -170,6 +173,7 @@ pub async fn sign_tx(
} else {
auxilary_data_vec = vec![None; user_details.programs_data.0.len()];
}

// gets fuel from chain
let max_instructions_per_programs_query =
entropy::storage().parameters().max_instructions_per_programs();
Expand All @@ -186,7 +190,9 @@ pub async fn sign_tx(
runtime.evaluate(&program, &signature_request, Some(&program_info.program_config), None)?;
}

let signers = get_signers_from_chain(&api, &rpc).await?;
let with_parent_key = user_details.derivation_path.is_some();
let signers = get_signers_from_chain(&api, &rpc, with_parent_key).await?;

// Use the validator info from chain as we can be sure it is in the correct order and the
// details are correct
user_sig_req.validators_info = signers;
Expand All @@ -207,22 +213,41 @@ pub async fn sign_tx(
request_author,
};

let _has_key = check_for_key(&string_verifying_key, &app_state.kv_store).await?;
// In the new registration flow we don't store the verifying key in the KVDB, so we only do this
// check if we're using the old registration flow
if user_details.derivation_path.is_none() {
let _has_key = check_for_key(&string_verifying_key, &app_state.kv_store).await?;
}

let derivation_path = if let Some(path) = user_details.derivation_path {
let decoded_path = String::decode(&mut path.as_ref())?;
let path = bip32::DerivationPath::from_str(&decoded_path)?;

Some(path)
} else {
None
};

let (mut response_tx, response_rx) = mpsc::channel(1);

// Do the signing protocol in another task, so we can already respond
tokio::spawn(async move {
let signing_protocol_output =
do_signing(&rpc, user_sig_req, &app_state, signing_session_id, request_limit)
.await
.map(|signature| {
(
BASE64_STANDARD.encode(signature.to_rsv_bytes()),
signer.signer().sign(&signature.to_rsv_bytes()),
)
})
.map_err(|error| error.to_string());
let signing_protocol_output = do_signing(
&rpc,
user_sig_req,
&app_state,
signing_session_id,
request_limit,
derivation_path,
)
.await
.map(|signature| {
(
BASE64_STANDARD.encode(signature.to_rsv_bytes()),
signer.signer().sign(&signature.to_rsv_bytes()),
)
})
.map_err(|error| error.to_string());

// This response chunk is sent later with the result of the signing protocol
if response_tx.try_send(serde_json::to_string(&signing_protocol_output)).is_err() {
Expand Down
2 changes: 2 additions & 0 deletions crates/threshold-signature-server/src/user/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ pub enum UserErr {
SubgroupGet(#[from] entropy_client::user::SubgroupGetError),
#[error("Unknown hashing algorthim - user is using a newer version than us")]
UnknownHashingAlgorithm,
#[error("Failed to derive BIP-32 account: {0}")]
Bip32DerivationError(#[from] bip32::Error),
}

impl From<hkdf::InvalidLength> for UserErr {
Expand Down
Loading

0 comments on commit 13a365f

Please sign in to comment.