Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Full transaction format with signer integration #74

Merged
merged 24 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
1c72ad7
Update ASN.1 schema to support full transaction.
koxu1996 Apr 22, 2024
d32ce25
Update `kairos-tx` schema to include new types.
koxu1996 Apr 22, 2024
9c2089c
Add converters for ASN.1 `Signature` and `PublicKey`.
koxu1996 Apr 22, 2024
34f3fee
Add constructor for ASN.1 `Transaction`.
koxu1996 Apr 22, 2024
e549cd6
Add `sha2` dependency.
koxu1996 Apr 22, 2024
171e5f9
Implement transaction hashing.
koxu1996 Apr 22, 2024
c3fe52c
Include `kairos-tx` in crypto library (feature flag).
koxu1996 Apr 22, 2024
c44fd5f
Add transaction related methods to crypto signer trait.
koxu1996 Apr 22, 2024
97adddb
Implement transaction signing and verification for Casper.
koxu1996 Apr 22, 2024
6c042d5
Fix clippy warnings and formatting.
koxu1996 Apr 22, 2024
c29f5bd
Add (de)serializer for full transaction.
koxu1996 Apr 22, 2024
b38f785
Merge remote-tracking branch 'origin/feature/kairos-tx-improvements' …
koxu1996 Apr 29, 2024
fca49f3
Allow cloning of new base types - signature and payload hash.
koxu1996 Apr 29, 2024
52b2854
Replace `make_tx` helper with `Transaction::new`.
koxu1996 Apr 29, 2024
8015e35
Move hashing function into `SigningPayload`.
koxu1996 Apr 29, 2024
3003e72
Add missing `PayloadHash` to transaction.
koxu1996 Apr 29, 2024
79c18c5
Warn about constructing transaction directly.
koxu1996 Apr 29, 2024
54616bc
Update incorrect comment in `sign_tx_payload()`.
koxu1996 Apr 29, 2024
31fc954
Fix formatting.
koxu1996 Apr 29, 2024
609fa94
Refactor encoding/decoding in `Transaction`.
koxu1996 Apr 29, 2024
69e1990
Fix formatting in crypto module.
koxu1996 Apr 29, 2024
0242e4c
Propagate error details in `CryptoError::TxHashingError`.
koxu1996 Apr 29, 2024
09cfc7e
Merge branch 'main' into feature/full-tx-format
marijanp Apr 29, 2024
578f520
Merge branch 'main' into feature/full-tx-format
koxu1996 Apr 30, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

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

4 changes: 3 additions & 1 deletion kairos-crypto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@ edition.workspace = true
license.workspace = true

[features]
default = ["crypto-casper"]
default = ["crypto-casper", "tx"]
crypto-casper = ["casper-types"]
tx = ["kairos-tx"]

[lib]

[dependencies]
hex = "0.4"
thiserror = "1"
kairos-tx = { path = "../kairos-tx", optional = true }

# Casper signer implementation.
casper-types = { version = "4", optional = true, features = ["std"] } # TODO: Change `std` -> `std-fs-io` in the future version.
9 changes: 9 additions & 0 deletions kairos-crypto/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,13 @@ pub enum CryptoError {
/// Private key is not provided.
#[error("private key is not provided")]
MissingPrivateKey,

/// Unable to compute transaction hash - invalid data given.
#[cfg(feature = "tx")]
#[error("unable to hash transaction data: {error}")]
TxHashingError { error: String },
/// Signing algorithm is not available in `kairos-tx`.
#[cfg(feature = "tx")]
#[error("algorithm not available in tx format")]
InvalidSigningAlgorithm,
}
44 changes: 44 additions & 0 deletions kairos-crypto/src/implementations/casper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,50 @@ impl CryptoSigner for Signer {

Ok(public_key)
}

#[cfg(feature = "tx")]
fn verify_tx(&self, tx: kairos_tx::asn::Transaction) -> Result<(), CryptoError> {
let tx_hash = tx.payload.hash().map_err(|e| CryptoError::TxHashingError {
error: e.to_string(),
})?;
let signature: Vec<u8> = tx.signature.into();
self.verify(tx_hash, signature)?;

Ok(())
}

#[cfg(feature = "tx")]
fn sign_tx_payload(
&self,
payload: kairos_tx::asn::SigningPayload,
) -> Result<kairos_tx::asn::Transaction, CryptoError> {
// Compute payload signature.
let tx_hash = payload.hash().map_err(|e| CryptoError::TxHashingError {
error: e.to_string(),
})?;
let signature = self.sign(tx_hash)?;

// Prepare public key.
let public_key = self.to_public_key()?;

// Prepare algorithm.
let algorithm = match self.public_key {
PublicKey::Ed25519(_) => Ok(kairos_tx::asn::SigningAlgorithm::CasperEd25519),
PublicKey::Secp256k1(_) => Ok(kairos_tx::asn::SigningAlgorithm::CasperSecp256k1),
_ => Err(CryptoError::InvalidSigningAlgorithm),
}?;

// Build full transaction.
let tx = kairos_tx::asn::Transaction::new(
public_key,
payload,
&tx_hash,
algorithm,
signature.into(),
);

Ok(tx)
}
}

#[cfg(test)]
Expand Down
8 changes: 8 additions & 0 deletions kairos-crypto/src/lib.rs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a benefit to this CryptoSigner trait. It should instead be replaced with a sign and verify generic function.
It also does too much at this point.

  1. from_private_key_file and from_public_key should be removed and replaced with From trait implementations
  2. to_public_key can be extracted to a From trait implementation too.
  3. sign and verify could be extracted in a trait, but I would be surprised that there is nothing like this already somewhere.

Instead of all these steps, I would remove the CryptoSigner trait completely, and implement a generic function that says "if these traits are implemented ..." I will produce either this signed result or an error.

sign_tx could moreover be an alias for calling sign(tx.into()). If the ASN1 type would have a respective From trait implementation. And would moreover not require us to introduce a tx feature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if from_private_key_file remain a method, not a From impl.
I don't like From/Into doing IO.

I agree we don't need the CryptoSigner trait, since there is only one implementation just keep the Signer struct.

Currently this implementation signs transaction hashes.
I have my doubts about signing transaction hashes rather than transactions bodies.
It is more general, but I suspect it's a lot of overhead in a zkVM on simple transfer/withdraw payloads.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have my doubts about signing transaction hashes rather than transactions bodies.

You have to hash the input before signing; signing algorithms take 255-bit input (for secp256k1) or 256-bit input (for ed25519). See https://crypto.stackexchange.com/a/59203

Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,13 @@ pub trait CryptoSigner {
signature_bytes: U,
) -> Result<(), CryptoError>;

#[cfg(feature = "tx")]
fn verify_tx(&self, tx: kairos_tx::asn::Transaction) -> Result<(), CryptoError>;
#[cfg(feature = "tx")]
fn sign_tx_payload(
&self,
payload: kairos_tx::asn::SigningPayload,
) -> Result<kairos_tx::asn::Transaction, CryptoError>;

Comment on lines +25 to +32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love feature gated parts of traits.
Generally they should be separated out into a separate trait.

I also don't like having a trait with a single impl, but these issues can be addressed in followup at a much latter date.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created follow up task - #86.

fn to_public_key(&self) -> Result<Vec<u8>, CryptoError>;
}
1 change: 1 addition & 0 deletions kairos-tx/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ license.workspace = true
[dependencies]
num-traits = "0.2"
rasn = { version = "0.12", default-features = false, features = ["macros"] }
sha2 = "0.10"

[dev-dependencies]
hex = "0.4"
Expand Down
19 changes: 19 additions & 0 deletions kairos-tx/schema.asn
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,28 @@ TxSchema DEFINITIONS AUTOMATIC TAGS ::= BEGIN

-- Basic types.
PublicKey ::= OCTET STRING
Signature ::= OCTET STRING
PayloadHash ::= OCTET STRING
Amount ::= INTEGER (0..18446744073709551615)
Nonce ::= INTEGER (0..18446744073709551615)

-- Full, top-level transaction type.
Transaction ::= SEQUENCE {
publicKey PublicKey,
payload SigningPayload,
hash PayloadHash,
algorithm SigningAlgorithm,
signature Signature,
...
}

-- Support for multiple signing algorithms.
SigningAlgorithm ::= ENUMERATED {
casperSecp256k1 (0),
casperEd25519 (1),
...
}

-- Transaction payload for signing.
SigningPayload ::= SEQUENCE {
nonce Nonce,
Expand Down
114 changes: 114 additions & 0 deletions kairos-tx/src/asn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub use rasn::types::{Integer, OctetString};
use num_traits::cast::ToPrimitive;
use rasn::types::AsnType;
use rasn::{Decode, Encode};
use sha2::Digest;

#[derive(AsnType, Encode, Decode, Debug, Clone)]
#[rasn(delegate)]
Expand All @@ -21,6 +22,12 @@ impl From<PublicKey> for Vec<u8> {
}
}

impl From<Vec<u8>> for PublicKey {
fn from(value: Vec<u8>) -> Self {
PublicKey(OctetString::copy_from_slice(&value))
}
}

impl From<&[u8]> for PublicKey {
fn from(value: &[u8]) -> Self {
PublicKey(OctetString::copy_from_slice(value))
Expand All @@ -33,6 +40,40 @@ impl<const N: usize> From<&[u8; N]> for PublicKey {
}
}

#[derive(AsnType, Encode, Decode, Debug, Clone)]
#[rasn(delegate)]
pub struct Signature(pub(crate) OctetString);

// Converts an ASN.1 decoded signature into raw byte representation.
impl From<Signature> for Vec<u8> {
fn from(value: Signature) -> Self {
value.0.into()
}
}

impl From<Vec<u8>> for Signature {
fn from(value: Vec<u8>) -> Self {
Signature(OctetString::copy_from_slice(&value))
}
}

#[derive(AsnType, Encode, Decode, Debug, Clone)]
#[rasn(delegate)]
pub struct PayloadHash(pub(crate) OctetString);

// Converts an ASN.1 decoded payload hash into raw byte representation.
impl From<PayloadHash> for Vec<u8> {
fn from(value: PayloadHash) -> Self {
value.0.into()
}
}

impl From<&[u8; 32]> for PayloadHash {
fn from(value: &[u8; 32]) -> Self {
PayloadHash(OctetString::copy_from_slice(value))
}
}

#[derive(AsnType, Encode, Decode, Debug, Clone)]
#[rasn(delegate)]
pub struct Amount(pub(crate) Integer);
Expand Down Expand Up @@ -82,6 +123,54 @@ impl From<u64> for Nonce {
}
}

#[derive(AsnType, Encode, Decode, Debug)]
#[non_exhaustive]
pub struct Transaction {
pub public_key: PublicKey, // NOTE: Field name can be different than defined in schema, as only **order** is crucial
pub payload: SigningPayload,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that you can compute the PayloadHash, I'm just wondering where the according computation happens? Or should it be added as a field here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching it, I forgot to add hash field - added in 3003e72.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Computing PayloadHash happens in crypto module in sign_tx_payload(). This is the only safe method of constructing fully validated transaction.

pub hash: PayloadHash,
pub algorithm: SigningAlgorithm,
pub signature: Signature,
}

impl Transaction {
/// Wraps full transaction data for storing.
///
/// CAUTION: This method does NOT perform validity checks - please use
/// `kairos-crypto::sign_tx_payload()` to construct it safely.
pub fn new(
public_key: impl Into<PublicKey>,
payload: SigningPayload,
hash: impl Into<PayloadHash>,
algorithm: SigningAlgorithm,
signature: Signature,
) -> Self {
Self {
public_key: public_key.into(),
payload,
hash: hash.into(),
algorithm,
signature,
}
}

pub fn der_encode(&self) -> Result<Vec<u8>, TxError> {
rasn::der::encode(self).map_err(TxError::EncodeError)
}

pub fn der_decode(value: impl AsRef<[u8]>) -> Result<Self, TxError> {
rasn::der::decode(value.as_ref()).map_err(TxError::DecodeError)
}
}

#[derive(AsnType, Encode, Decode, Debug, PartialEq, Copy, Clone)]
#[rasn(enumerated)]
#[non_exhaustive]
pub enum SigningAlgorithm {
CasperSecp256k1 = 0,
CasperEd25519 = 1,
}

#[derive(AsnType, Encode, Decode, Debug)]
#[non_exhaustive]
pub struct SigningPayload {
Expand Down Expand Up @@ -130,6 +219,15 @@ impl SigningPayload {
pub fn der_decode(value: impl AsRef<[u8]>) -> Result<Self, TxError> {
rasn::der::decode(value.as_ref()).map_err(TxError::DecodeError)
}

// Computes the hash for a transaction.
// Hash is obtained from payload by computing sha256 of DER encoded ASN.1 data.
pub fn hash(&self) -> Result<[u8; 32], TxError> {
let data = self.der_encode()?;
let tx_hash: [u8; 32] = sha2::Sha256::digest(data).into();

Ok(tx_hash)
}
}

#[derive(AsnType, Encode, Decode, Debug)]
Expand Down Expand Up @@ -206,6 +304,22 @@ impl Withdrawal {
}
}

impl TryFrom<&[u8]> for Transaction {
type Error = TxError;

fn try_from(value: &[u8]) -> Result<Self, Self::Error> {
Transaction::der_decode(value)
}
}

impl TryFrom<Transaction> for Vec<u8> {
type Error = TxError;

fn try_from(value: Transaction) -> Result<Self, Self::Error> {
value.der_encode()
}
}

impl TryFrom<&[u8]> for SigningPayload {
type Error = TxError;

Expand Down
Loading