Skip to content

Commit

Permalink
remove unwraps (#350)
Browse files Browse the repository at this point in the history
* remove unwraps

* remove more unwraps

* remove unwraps from validator folder

* remove unwrap from constraints

* remove unwraps from sig init

* pipeline fix

* pipeline

* validation unwraps removed

* pipeline

* remove unwraps from do sign

* lint

* duplicate error fix

* clean todos

---------

Co-authored-by: jesse <jesse@entropy.wxy>
  • Loading branch information
JesseAbram and jesse authored May 31, 2023
1 parent f24eb68 commit 646914a
Show file tree
Hide file tree
Showing 21 changed files with 205 additions and 164 deletions.
62 changes: 3 additions & 59 deletions .circleci/then.yml
Original file line number Diff line number Diff line change
@@ -1,21 +1,4 @@
commands:
dl-and-install:
steps:
- run: |
echo $BKEY>privatekey
wget https://testing.entropy.family/releases/bi/linux-amd64.tar.zst
wget https://testing.entropy.family/releases/bi/linux-amd64.tar.zst.sha256sum
export CHECKSUM_BI=$(sha256sum linux-amd64.tar.zst|cut -c -64)
export TARBALL="entropy-$(git tag|tail -n 1)-$(git rev-parse --short HEAD).tar.zst"
if [ "$CHECKSUM_BI" != "68b54972fb9e78167923b7ba66f8fc34642744c9bacdcfc809f3ae41f9cb82b1" ]; then echo -e "bad checksum \n\twanted 68b54972fb9e78167923b7ba66f8fc34642744c9bacdcfc809f3ae41f9cb82b1\n\tgot $CHECKSUM_BI\n && exit 0"; fi
tar xvf linux-amd64.tar.zst && sudo mv linux-amd64/bi /bin
cat privatekey| tr -d '\n'|bi pub|tr -d '\n'>publickey
echo "signing with publickey: $(cat publickey)"
echo "key id: $(cat privatekey|sha256sum|cut -c -64)"
bi sign privatekey publickey >publickey.sig
echo "test signing public key:"
bi verify publickey.sig publickey publickey
bi -tor=false send publickey.sig publickey publickey https://testing.entropy.family/u
fmt-lint:
steps:
- run: curl -LsSf https://github.com/tamasfe/taplo/releases/download/0.8.0/taplo-full-linux-x86_64.gz | gunzip -N -d - > ${CARGO_HOME:-~/.cargo}/bin/taplo && chmod +x ${CARGO_HOME:-~/.cargo}/bin/taplo
Expand Down Expand Up @@ -51,45 +34,13 @@ commands:
- install-dependencies
- checkout
- install-dependencies
- dl-and-install
- install-rust
new-cmd:
steps:
- run: echo test
build:
steps:
- run: cargo build --release
release:
steps:
- run: |
OUTDIR="entropy"
VERSION="$(git tag|tail -n 1)-$(git rev-parse --short HEAD)"
TARBALL="$OUTDIR.tar.zst"
mkdir -p $OUTDIR
echo $TARBALL > $OUTDIR/VERSION
cat LICENSE > $OUTDIR/LICENSE
cp target/release/entropy target/release/server $OUTDIR
tar acvf $TARBALL $OUTDIR
bi -tor=false sign privatekey $TARBALL > $TARBALL.sig
bi -tor=false send $TARBALL.sig $TARBALL publickey https://testing.entropy.family/u ci-latest > artifact-url
sign-and-upload-pr:
steps:
- run: |
OUTDIR="entropy-$(git tag|tail -n 1|sed 's/\./-/g')-$(git rev-parse --short HEAD)"
echo -e "## $(git tag|tail -n 1)-$(git rev-parse --short HEAD)\n" > pr-comment
echo -e "\nnice work, looks like everything passed."
TARBALL="$OUTDIR.tar.zst"
mkdir -p $OUTDIR
echo $TARBALL >$OUTDIR/version
cp target/release/entropy target/release/server $OUTDIR
tar acvf $TARBALL $OUTDIR
bi -tor=false sign privatekey $TARBALL > $TARBALL.sig
bi -tor=false send $TARBALL.sig $TARBALL publickey https://testing.entropy.family/u ci-latest > artifact-url
echo -e "\nyour build artifacts are available [here](cat artifact-url).\n" >> pr-comment
- store_artifacts:
path: artifact-url
destination: artifact-url
comment-on-pr:
steps:
- run: |
Expand All @@ -107,7 +58,7 @@ commands:
--data-raw "'{
\"body\": \"$(cat pr-comment)\"
}'"
jobs:
crypto-server:
machine:
Expand Down Expand Up @@ -140,8 +91,7 @@ jobs:
- run:
command: pushd node && cargo build --all-targets --release -j $(nproc) && cargo test --all-targets --release
no_output_timeout: 45m
- sign-and-upload-pr
build-and-release:
build-and-release:
machine:
image: ubuntu-2204:2022.10.2
resource_class: xlarge
Expand All @@ -150,7 +100,7 @@ jobs:
- install-dependencies-and-checkout
- build
- release

parameters:
crypto:
default: false
Expand All @@ -166,12 +116,6 @@ parameters:
type: boolean
version: 2.1
workflows:
release:
when:
and:
- equal: [ master, << pipeline.git.branch >> ]
jobs:
- build-and-release
lint:
jobs:
- fmt-lint-all
Expand Down
6 changes: 5 additions & 1 deletion crypto/constraints/src/constraints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ impl Evaluate<Evm> for Acl<[u8; 20]> {
let converted_addresses: Vec<NameOrAddress> =
self.addresses.into_iter().map(|a| NameOrAddress::Address(H160::from(a))).collect();

match (converted_addresses.contains(&tx.to.unwrap()), self.kind) {
match (
converted_addresses
.contains(&tx.to.ok_or(Error::Evaluation("Error Parsing to address"))?),
self.kind,
) {
(true, AclKind::Allow) => Ok(()),
(false, AclKind::Deny) => Ok(()),
_ => Err(Error::Evaluation("Transaction not allowed.")),
Expand Down
4 changes: 2 additions & 2 deletions crypto/server/src/helpers/launch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use kvdb::{
use serde::Deserialize;
use subxt::ext::sp_core::{crypto::AccountId32, sr25519, Pair};

use crate::message::{derive_static_secret, mnemonic_to_pair};
use crate::validation::{derive_static_secret, mnemonic_to_pair};

pub const DEFAULT_MNEMONIC: &str =
"alarm mutual concert decrease hurry invest culture survey diagram crash snap click";
Expand Down Expand Up @@ -124,7 +124,7 @@ pub async fn setup_mnemonic(kv: &KvManager, is_alice: bool, is_bob: bool) -> Res

let phrase = mnemonic.phrase();
println!("[server-config]");
let pair = mnemonic_to_pair(&mnemonic);
let pair = mnemonic_to_pair(&mnemonic).expect("Issue deriving Mnemonic");
let static_secret = derive_static_secret(&pair);
let dh_public = x25519_dalek::PublicKey::from(&static_secret);

Expand Down
46 changes: 28 additions & 18 deletions crypto/server/src/helpers/signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,18 @@ use std::{collections::HashMap, sync::Mutex};
use bip39::{Language, Mnemonic};
use kvdb::kv_manager::{KvManager, PartyId};
use rocket::{http::Status, State};
use sp_core::crypto::AccountId32;
use synedrion::k256::ecdsa::{RecoveryId, Signature};

use crate::{
get_signer,
message::mnemonic_to_pair,
sign_init::SignInit,
signing_client::{
new_party::{Channels, ThresholdSigningService},
subscribe::{subscribe_to_them, Listener},
SignerState, SigningErr,
},
validation::mnemonic_to_pair,
};

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -72,10 +73,16 @@ pub async fn do_signing(
signatures: &State<SignatureState>,
tx_id: String,
) -> Result<Status, SigningErr> {
let info = SignInit::new(message.clone(), tx_id);
let info = SignInit::new(message.clone(), tx_id)?;
let signing_service = ThresholdSigningService::new(state, kv_manager);

let my_id = PartyId::new(get_signer(kv_manager).await.unwrap().account_id().clone());
let my_id = PartyId::new(
get_signer(kv_manager)
.await
.map_err(|_| SigningErr::UserError("Error getting Signer"))?
.account_id()
.clone(),
);

// set up context for signing protocol execution
let sign_context = signing_service.get_sign_context(info.clone()).await?;
Expand All @@ -85,7 +92,7 @@ pub async fn do_signing(
state
.listeners
.lock()
.expect("lock shared data")
.map_err(|_| SigningErr::SessionError("Error getting lock".to_string()))?
// TODO: using signature ID as session ID. Correct?
.insert(sign_context.sign_init.sig_uid.clone(), listener);
let channels = {
Expand All @@ -94,26 +101,29 @@ pub async fn do_signing(
Channels(broadcast_out, stream_in)
};

let raw = kv_manager.kv().get("MNEMONIC").await.unwrap();
let secret = core::str::from_utf8(&raw).unwrap();
let mnemonic = Mnemonic::from_phrase(secret, Language::English).unwrap();
let threshold_signer = mnemonic_to_pair(&mnemonic);
let tss_accounts = message
let raw = kv_manager.kv().get("MNEMONIC").await?;
let secret = core::str::from_utf8(&raw)?;
let mnemonic = Mnemonic::from_phrase(secret, Language::English)
.map_err(|e| SigningErr::Mnemonic(e.to_string()))?;
let threshold_signer =
mnemonic_to_pair(&mnemonic).map_err(|_| SigningErr::SecretString("Secret String Error"))?;
let tss_accounts_results: Result<Vec<AccountId32>, SigningErr> = message
.validators_info
.iter()
.map(|validator_info| {
let address_slice: &[u8; 32] = &validator_info
.tss_account
.clone()
.try_into()
.expect("slice with incorrect length");
sp_core::crypto::AccountId32::new(*address_slice)
let address_slice: &[u8; 32] =
&validator_info.tss_account.clone().try_into().map_err(|_| {
SigningErr::AddressConversionError("Invalid Length".to_string())
})?;
Ok(sp_core::crypto::AccountId32::new(*address_slice))
})
.collect::<Vec<_>>();
.collect();

let tss_accounts = tss_accounts_results.map_err(SigningErr::from)?;

let result = signing_service
.execute_sign(&sign_context, channels, &threshold_signer, tss_accounts)
.await
.unwrap();
.await?;

signing_service.handle_result(&result, message.sig_request.sig_hash.as_slice(), signatures);

Expand Down
2 changes: 1 addition & 1 deletion crypto/server/src/helpers/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ use crate::{
signing::SignatureState,
substrate::{get_subgroup, make_register},
},
message::SignedMessage,
new_user,
r#unsafe::api::{delete, get, put, remove_keys},
sign_tx,
Expand All @@ -37,6 +36,7 @@ use crate::{
SignerState,
},
store_tx,
validation::SignedMessage,
validator::api::sync_kvdb,
};

Expand Down
2 changes: 1 addition & 1 deletion crypto/server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
pub(crate) mod chain_api;
pub(crate) mod health;
mod helpers;
pub(crate) mod message;
pub(crate) mod sign_init;
mod signing_client;
mod r#unsafe;
mod user;
pub(crate) mod validation;
mod validator;
use rocket::{
fairing::{Fairing, Info, Kind},
Expand Down
27 changes: 17 additions & 10 deletions crypto/server/src/sign_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use entropy_shared::Message;
use serde::{Deserialize, Serialize};
use synedrion::sessions::PrehashedMessage;

use crate::signing_client::SigningErr;
/// Information passed to the Signing Client, to initiate the signing process.
/// Most of this information comes from a `Message` struct which gets propagated when a user's
/// signature request transaction is included in a finalized block.
Expand All @@ -19,20 +20,26 @@ pub struct SignInit {
}

impl SignInit {
// TODO(TK): option to make msg Bytes, and have `new` do input validation
// todo: placeholder for actual logic
/// Creates new signing object based on passed in data
#[allow(dead_code)]
pub fn new(message: Message, tx_id: String) -> Self {
let digest: PrehashedMessage = message.sig_request.sig_hash.as_slice().try_into().unwrap();
pub fn new(message: Message, tx_id: String) -> Result<Self, SigningErr> {
let digest: PrehashedMessage = message.sig_request.sig_hash.as_slice().try_into()?;
let raw_address = &message.account;
let address_slice: &[u8; 32] =
&raw_address.clone().try_into().expect("slice with incorrect length");
let address_slice: &[u8; 32] = &raw_address
.clone()
.try_into()
.map_err(|_| SigningErr::AddressConversionError("Invalid Length".to_string()))?;
let user = sp_core::crypto::AccountId32::new(*address_slice);
let ip_addresses = message
let ip_addresses_results: Result<Vec<String>, _> = message
.validators_info
.into_iter()
.map(|validator_info| String::from_utf8(validator_info.ip_address).unwrap())
.collect::<Vec<_>>();
Self { sig_uid: tx_id, msg: digest, substrate_key: user.to_string(), ip_addresses }
.map(|validator_info| {
let ip_address = String::from_utf8(validator_info.ip_address)?;
Ok::<std::string::String, SigningErr>(ip_address)
})
.collect();
let ip_addresses = ip_addresses_results.map_err(SigningErr::from)?;

Ok(Self { sig_uid: tx_id, msg: digest, substrate_key: user.to_string(), ip_addresses })
}
}
2 changes: 1 addition & 1 deletion crypto/server/src/signing_client/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub async fn subscribe_to_me(

let party_id = msg.party_id().map_err(SubscribeErr::InvalidPartyId)?;

if !state.contains_listener(&msg.session_id) {
if !state.contains_listener(&msg.session_id)? {
// Chain node hasn't yet informed this node of the party. Wait for a timeout and procede (or
// fail below)
tokio::time::sleep(std::time::Duration::from_secs(SUBSCRIBE_TIMEOUT_SECONDS)).await;
Expand Down
24 changes: 22 additions & 2 deletions crypto/server/src/signing_client/errors.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Errors for everyone ✅
use std::io::Cursor;
use std::{io::Cursor, string::FromUtf8Error};

use kvdb::kv_manager::error::InnerKvError;
use rocket::{
Expand Down Expand Up @@ -44,6 +44,10 @@ pub enum SigningErr {
AddressConversionError(String),
#[error("reqwest error: {0}")]
Reqwest(#[from] reqwest::Error),
#[error("Utf8Error: {0:?}")]
Utf8(#[from] std::str::Utf8Error),
#[error("reqwest event error: {0}")]
ReqwestEvent(#[from] reqwest_eventsource::Error),
#[error("Broadcast error: {0}")]
Broadcast(#[from] Box<tokio::sync::broadcast::error::SendError<SigningMessage>>),
#[error("anyhow error: {0}")]
Expand All @@ -59,7 +63,21 @@ pub enum SigningErr {
#[error("Serde Json error: {0}")]
SerdeJson(#[from] serde_json::Error),
#[error("Message validation Error: {0}")]
MessageValidation(&'static str),
MessageValidation(String),
#[error("Cannont clone request: {0}")]
CannotCloneRequest(String),
#[error("Unexpected event: {0}")]
UnexpectedEvent(String),
#[error("Session Error: {0}")]
SessionError(String),
#[error("String Conversion Error: {0}")]
StringConversion(#[from] FromUtf8Error),
#[error("Secret String failure: {0:?}")]
SecretString(&'static str),
#[error("User Error: {0}")]
UserError(&'static str),
#[error("mnemonic failure: {0:?}")]
Mnemonic(String),
}

impl<'r, 'o: 'r> Responder<'r, 'o> for SigningErr {
Expand All @@ -84,6 +102,8 @@ pub enum SubscribeErr {
// Validation(&'static str),
#[error("invalid party ID: {0}")]
InvalidPartyId(String),
#[error("Lock Error: {0}")]
LockError(String),
}

// todo: delete
Expand Down
8 changes: 6 additions & 2 deletions crypto/server/src/signing_client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ pub struct SignerState {

impl SignerState {
/// Create a new `SignerState`
pub fn contains_listener(&self, session_id: &String) -> bool {
self.listeners.lock().unwrap().contains_key(session_id)
pub fn contains_listener(&self, session_id: &String) -> Result<bool, SubscribeErr> {
Ok(self
.listeners
.lock()
.map_err(|e| SubscribeErr::LockError(e.to_string()))?
.contains_key(session_id))
}
}
Loading

0 comments on commit 646914a

Please sign in to comment.