From 3936d2dd84b29157cbf4aa578cc231621a73460c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tuomas=20M=C3=A4kinen?= <1947505+tuommaki@users.noreply.github.com> Date: Sun, 21 Jan 2024 17:05:02 +0200 Subject: [PATCH] Add transaction validation (#36) This is the first step towards having validation for transactions. It verifies that transaction signature is correct and that current workflow restrictions are checked. The transaction is validated on entering mempool, which covers currently all the key use cases in the program. --- crates/cli/src/lib.rs | 23 ++-- crates/node/src/mempool/mod.rs | 3 + crates/node/src/networking/p2p.rs | 14 +- crates/node/src/scheduler/mod.rs | 28 ++-- .../node/src/storage/database/entity/mod.rs | 2 + .../src/storage/database/entity/public_key.rs | 89 +++++++++++++ .../storage/database/entity/transaction.rs | 4 + crates/node/src/storage/database/postgres.rs | 3 + crates/node/src/types/transaction.rs | 121 ++++++++++++++++-- crates/node/src/workflow/mod.rs | 58 +++------ 10 files changed, 248 insertions(+), 97 deletions(-) create mode 100644 crates/node/src/storage/database/entity/public_key.rs diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index aeabdf19..89b7b279 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -123,15 +123,12 @@ pub async fn run_exec_command( .map(|t| t.try_into()) .collect::, _>>()?; - let mut tx = Transaction { - payload: Payload::Run { + let tx = Transaction::new( + Payload::Run { workflow: Workflow { steps }, }, - // NOTE: In the devnet `nonce` is just a placeholder for the future. - nonce: 42, - ..Default::default() - }; - tx.sign(&key); + &key, + ); client.send_transaction(&tx).await?; Ok(tx.hash.to_string()) @@ -204,18 +201,14 @@ pub async fn run_deploy_command( let prover_hash = prover_data.hash.to_string(); let verifier_hash = verifier_data.hash.to_string(); - let mut tx = Transaction { - payload: Payload::Deploy { + let tx = Transaction::new( + Payload::Deploy { name, prover: prover_data, verifier: verifier_data, }, - nonce: 42, - ..Default::default() - }; - - // Transaction hash gets computed during this as well. - tx.sign(&key); + &key, + ); client .send_transaction(&tx) diff --git a/crates/node/src/mempool/mod.rs b/crates/node/src/mempool/mod.rs index 285250c5..e8cf46f9 100644 --- a/crates/node/src/mempool/mod.rs +++ b/crates/node/src/mempool/mod.rs @@ -51,6 +51,9 @@ impl Mempool { } pub async fn add(&mut self, tx: Transaction) -> Result<()> { + // First validate transaction. + tx.validate()?; + let mut tx = tx; self.storage.set(&tx).await?; diff --git a/crates/node/src/networking/p2p.rs b/crates/node/src/networking/p2p.rs index 0f849feb..797e8238 100644 --- a/crates/node/src/networking/p2p.rs +++ b/crates/node/src/networking/p2p.rs @@ -151,9 +151,9 @@ impl Writing for P2P { mod tests { use super::*; use eyre::Result; - use gevulot_node::types::{transaction::Payload, Hash, Transaction}; + use gevulot_node::types::{transaction::Payload, Transaction}; use libsecp256k1::SecretKey; - use rand::{rngs::StdRng, RngCore, SeedableRng}; + use rand::{rngs::StdRng, SeedableRng}; use tokio::sync::mpsc::{self, Sender}; use tracing::level_filters::LevelFilter; use tracing_subscriber::EnvFilter; @@ -222,16 +222,8 @@ mod tests { fn new_tx() -> Transaction { let rng = &mut StdRng::from_entropy(); - let mut tx = Transaction { - hash: Hash::random(rng), - payload: Payload::Empty, - nonce: rng.next_u64(), - ..Default::default() - }; - let key = SecretKey::random(rng); - tx.sign(&key); - tx + Transaction::new(Payload::Empty, &key) } fn start_logger(default_level: LevelFilter) { diff --git a/crates/node/src/scheduler/mod.rs b/crates/node/src/scheduler/mod.rs index dc20be8a..47bdb84e 100644 --- a/crates/node/src/scheduler/mod.rs +++ b/crates/node/src/scheduler/mod.rs @@ -8,7 +8,7 @@ use crate::vmm::{vm_server::TaskManager, VMId}; use crate::workflow::{WorkflowEngine, WorkflowError}; use async_trait::async_trait; use gevulot_node::types::transaction::Payload; -use gevulot_node::types::{Signature, TaskKind, Transaction}; +use gevulot_node::types::{TaskKind, Transaction}; use libsecp256k1::SecretKey; pub use program_manager::ProgramManager; use rand::RngCore; @@ -294,29 +294,23 @@ impl TaskManager for Scheduler { ); let nonce = rand::thread_rng().next_u64(); - let mut tx = match running_task.task.kind { - TaskKind::Proof => Transaction { - hash: Hash::default(), - payload: Payload::Proof { + let tx = match running_task.task.kind { + TaskKind::Proof => Transaction::new( + Payload::Proof { parent: running_task.task.tx, prover: program, proof: result.data, }, - nonce, - signature: Signature::default(), - propagated: false, - }, - TaskKind::Verification => Transaction { - hash: Hash::default(), - payload: Payload::Verification { + &self.node_key, + ), + TaskKind::Verification => Transaction::new( + Payload::Verification { parent: running_task.task.tx, verifier: program, verification: result.data, }, - nonce, - signature: Signature::default(), - propagated: false, - }, + &self.node_key, + ), TaskKind::PoW => { todo!("proof of work tasks not implemented yet"); } @@ -328,8 +322,6 @@ impl TaskManager for Scheduler { } }; - tx.sign(&self.node_key); - let mut mempool = self.mempool.write().await; if let Err(err) = mempool.add(tx.clone()).await { tracing::error!("failed to add transaction to mempool: {}", err); diff --git a/crates/node/src/storage/database/entity/mod.rs b/crates/node/src/storage/database/entity/mod.rs index 97e799c1..2a14e19a 100644 --- a/crates/node/src/storage/database/entity/mod.rs +++ b/crates/node/src/storage/database/entity/mod.rs @@ -1,4 +1,6 @@ pub mod payload; +pub mod public_key; pub mod transaction; +pub use public_key::PublicKey; pub use transaction::Transaction; diff --git a/crates/node/src/storage/database/entity/public_key.rs b/crates/node/src/storage/database/entity/public_key.rs new file mode 100644 index 00000000..4f0fc6c0 --- /dev/null +++ b/crates/node/src/storage/database/entity/public_key.rs @@ -0,0 +1,89 @@ +use std::fmt; + +use serde::{Deserialize, Serialize}; +use sqlx::{Decode, Encode, Postgres, Type}; +use thiserror::Error; + +#[allow(clippy::enum_variant_names)] +#[derive(Error, Debug)] +pub enum PublicKeyError { + #[error("public key parse error: {0}")] + ParseError(String), +} + +#[derive(Clone, Debug, Deserialize, Serialize)] +pub struct PublicKey(pub libsecp256k1::PublicKey); + +impl Default for PublicKey { + fn default() -> Self { + PublicKey(libsecp256k1::PublicKey::from_secret_key( + &libsecp256k1::SecretKey::default(), + )) + } +} + +impl fmt::Display for PublicKey { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", hex::encode(self.0.serialize())) + } +} +impl From for libsecp256k1::PublicKey { + fn from(value: PublicKey) -> Self { + value.0 + } +} + +impl From for PublicKey { + fn from(value: String) -> Self { + Self::try_from(value.as_str()).expect("from string") + } +} + +impl TryFrom<&str> for PublicKey { + type Error = PublicKeyError; + + fn try_from(value: &str) -> Result { + let bs = hex::decode(value).map_err(|e| PublicKeyError::ParseError(e.to_string()))?; + let bs: [u8; 65] = bs + .try_into() + .map_err(|_| PublicKeyError::ParseError("byte array length".to_string()))?; + let pub_key = libsecp256k1::PublicKey::parse(&bs) + .map_err(|e| PublicKeyError::ParseError(e.to_string()))?; + Ok(PublicKey(pub_key)) + } +} + +impl PublicKey { + pub fn from_secret_key(secret_key: &libsecp256k1::SecretKey) -> PublicKey { + PublicKey(libsecp256k1::PublicKey::from_secret_key(secret_key)) + } +} + +impl<'q> Decode<'q, Postgres> for PublicKey { + fn decode( + value: >::ValueRef, + ) -> Result { + let str_value = >::decode(value)?; + Ok(PublicKey::from(str_value)) + } +} + +impl<'q> Encode<'q, Postgres> for PublicKey { + fn encode_by_ref( + &self, + buf: &mut >::ArgumentBuffer, + ) -> sqlx::encode::IsNull { + let str_value = self.to_string(); + >::encode(str_value, buf) + } +} + +impl Type for PublicKey { + fn type_info() -> ::TypeInfo { + >::type_info() + } + + fn compatible(ty: &::TypeInfo) -> bool { + >::compatible(ty) + } +} diff --git a/crates/node/src/storage/database/entity/transaction.rs b/crates/node/src/storage/database/entity/transaction.rs index c3a8b255..1e526b0d 100644 --- a/crates/node/src/storage/database/entity/transaction.rs +++ b/crates/node/src/storage/database/entity/transaction.rs @@ -1,3 +1,4 @@ +use crate::storage::database::entity; use crate::types::{self, transaction, Hash, Signature}; use serde::{Deserialize, Serialize}; @@ -19,6 +20,7 @@ pub enum Kind { #[derive(Clone, Debug, Default, Deserialize, Serialize, sqlx::FromRow)] pub struct Transaction { + pub author: entity::PublicKey, pub hash: Hash, pub kind: Kind, pub nonce: sqlx::types::Decimal, @@ -42,6 +44,7 @@ impl From<&types::Transaction> for Transaction { }; Transaction { + author: entity::PublicKey(value.author), hash: value.hash, kind, nonce: value.nonce.into(), @@ -54,6 +57,7 @@ impl From<&types::Transaction> for Transaction { impl From for types::Transaction { fn from(value: Transaction) -> types::Transaction { types::Transaction { + author: value.author.into(), hash: value.hash, // This field is complemented separately. payload: transaction::Payload::Empty, diff --git a/crates/node/src/storage/database/postgres.rs b/crates/node/src/storage/database/postgres.rs index e9c0b571..50ab9891 100644 --- a/crates/node/src/storage/database/postgres.rs +++ b/crates/node/src/storage/database/postgres.rs @@ -517,6 +517,8 @@ impl Database { #[cfg(test)] mod tests { + use libsecp256k1::{PublicKey, SecretKey}; + use crate::types::{ transaction::{Payload, ProgramMetadata}, Signature, Transaction, @@ -532,6 +534,7 @@ mod tests { .expect("failed to connect to db"); let tx = Transaction { + author: PublicKey::from_secret_key(&SecretKey::default()), hash: Hash::default(), payload: Payload::Deploy { name: "test deployment".to_string(), diff --git a/crates/node/src/types/transaction.rs b/crates/node/src/types/transaction.rs index 5c1de4d9..84373c51 100644 --- a/crates/node/src/types/transaction.rs +++ b/crates/node/src/types/transaction.rs @@ -1,11 +1,13 @@ -use std::sync::Arc; +use std::{collections::HashSet, sync::Arc}; use super::hash::Hash; use super::signature::Signature; +use eyre::Result; use libsecp256k1::{sign, verify, Message, PublicKey, SecretKey}; use num_bigint::BigInt; use serde::{Deserialize, Serialize}; use sha3::{Digest, Sha3_256}; +use thiserror::Error; #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] pub enum TransactionTree { @@ -223,8 +225,16 @@ impl Payload { } } -#[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq)] +#[allow(clippy::enum_variant_names)] +#[derive(Error, Debug)] +pub enum TransactionError { + #[error("validation: {0}")] + Validation(String), +} + +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] pub struct Transaction { + pub author: PublicKey, pub hash: Hash, pub payload: Payload, pub nonce: u64, @@ -233,7 +243,37 @@ pub struct Transaction { pub propagated: bool, } +impl Default for Transaction { + fn default() -> Self { + Self { + author: PublicKey::from_secret_key(&SecretKey::default()), + hash: Hash::default(), + payload: Payload::default(), + nonce: 0, + signature: Signature::default(), + propagated: false, + } + } +} + impl Transaction { + pub fn new(payload: Payload, signing_key: &SecretKey) -> Self { + let author = PublicKey::from_secret_key(signing_key); + + let mut tx = Self { + author, + hash: Hash::default(), + payload, + nonce: 0, + signature: Signature::default(), + propagated: false, + }; + + tx.sign(signing_key); + + tx + } + pub fn sign(&mut self, key: &SecretKey) { // Refresh transaction hash before signing. self.hash = self.compute_hash(); @@ -242,10 +282,10 @@ impl Transaction { self.signature = sig.into(); } - pub fn verify(&self, pub_key: &PublicKey) -> bool { + pub fn verify(&self) -> bool { let hash = self.compute_hash(); let msg: Message = hash.into(); - verify(&msg, &self.signature.into(), pub_key) + verify(&msg, &self.signature.into(), &self.author) } pub fn compute_hash(&self) -> Hash { @@ -256,6 +296,30 @@ impl Transaction { hasher.update(self.nonce.to_be_bytes()); (&hasher.finalize()[0..32]).into() } + + pub fn validate(&self) -> Result<()> { + if let Payload::Run { ref workflow } = self.payload { + let mut programs = HashSet::new(); + for step in &workflow.steps { + if !programs.insert(step.program) { + return Err(TransactionError::Validation(format!( + "multiple programs in workflow: {}", + &step.program + )) + .into()); + } + } + } + + if !self.verify() { + return Err(TransactionError::Validation(String::from( + "signature verification failed", + )) + .into()); + } + + Ok(()) + } } #[cfg(test)] @@ -268,25 +332,58 @@ mod tests { #[test] fn test_sign_and_verify_tx() { let sk = SecretKey::random(&mut StdRng::from_entropy()); - let pk = PublicKey::from_secret_key(&sk); - let mut tx = Transaction::default(); - tx.sign(&sk); - assert!(tx.verify(&pk)); + let tx = Transaction::new(Payload::Empty, &sk); + assert!(tx.verify()); } #[test] fn test_verify_fails_on_tamper() { let sk = SecretKey::random(&mut StdRng::from_entropy()); - let pk = PublicKey::from_secret_key(&sk); - let mut tx = Transaction::default(); - tx.sign(&sk); + let mut tx = Transaction::new(Payload::Empty, &sk); // Change nonce after signing. tx.nonce += 1; // Verify must return false. - assert!(!tx.verify(&pk)); + assert!(!tx.verify()); + } + + #[test] + fn test_tx_validate_ensures_unique_programs() { + let prover = WorkflowStep::default(); + let verifier = WorkflowStep::default(); + + let workflow = Workflow { + // Both steps are `Default::default()` -> same program hash -> invalid. + steps: vec![prover, verifier], + }; + + let sk = SecretKey::random(&mut StdRng::from_entropy()); + let tx = Transaction { + author: PublicKey::from_secret_key(&sk), + hash: Hash::default(), + payload: Payload::Run { workflow }, + nonce: 0, + signature: Signature::default(), + propagated: false, + }; + + assert!(tx.validate().is_err()); + } + + #[test] + fn test_tx_validations_verifies_signature() { + let tx = Transaction { + author: PublicKey::from_secret_key(&SecretKey::default()), + hash: Hash::default(), + payload: Payload::Empty, + nonce: 0, + signature: Signature::default(), + propagated: false, + }; + + assert!(tx.validate().is_err()); } } diff --git a/crates/node/src/workflow/mod.rs b/crates/node/src/workflow/mod.rs index 79c6891d..89c0c9ac 100644 --- a/crates/node/src/workflow/mod.rs +++ b/crates/node/src/workflow/mod.rs @@ -328,7 +328,7 @@ mod tests { use gevulot_node::types::{ transaction::{Payload, ProgramData, Workflow, WorkflowStep}, - Hash, Signature, TaskKind, Transaction, + Hash, TaskKind, Transaction, }; use libsecp256k1::SecretKey; use rand::{rngs::StdRng, SeedableRng}; @@ -444,70 +444,46 @@ mod tests { fn transaction_for_workflow_steps(steps: Vec) -> Transaction { let key = SecretKey::random(&mut StdRng::from_entropy()); - let mut tx = Transaction { - hash: Hash::default(), - payload: Payload::Run { + Transaction::new( + Payload::Run { workflow: Workflow { steps }, }, - nonce: 1, - signature: Signature::default(), - propagated: false, - }; - - tx.sign(&key); - tx + &key, + ) } fn transaction_for_proof(parent: &Hash, program: &Hash) -> Transaction { let key = SecretKey::random(&mut StdRng::from_entropy()); - let mut tx = Transaction { - hash: Hash::default(), - payload: Payload::Proof { + Transaction::new( + Payload::Proof { parent: *parent, prover: *program, proof: "proof.".into(), }, - nonce: 1, - signature: Signature::default(), - propagated: false, - }; - - tx.sign(&key); - tx + &key, + ) } fn transaction_for_proofkey(parent: &Hash) -> Transaction { let key = SecretKey::random(&mut StdRng::from_entropy()); - let mut tx = Transaction { - hash: Hash::default(), - payload: Payload::ProofKey { + Transaction::new( + Payload::ProofKey { parent: *parent, key: "key.".into(), }, - nonce: 1, - signature: Signature::default(), - propagated: false, - }; - - tx.sign(&key); - tx + &key, + ) } fn transaction_for_verification(parent: &Hash, program: &Hash) -> Transaction { let key = SecretKey::random(&mut StdRng::from_entropy()); - let mut tx = Transaction { - hash: Hash::default(), - payload: Payload::Verification { + Transaction::new( + Payload::Verification { parent: *parent, verifier: *program, verification: b"verification.".to_vec(), }, - nonce: 1, - signature: Signature::default(), - propagated: false, - }; - - tx.sign(&key); - tx + &key, + ) } }