Skip to content

Commit

Permalink
refactor(proto)!: update SignedTransaction to contain Any for tra…
Browse files Browse the repository at this point in the history
…nsaction (#1044)

## Summary
as title says

## Background
protobuf encoding is not deterministic, so we want to verify the
signature over the exact same bytes signed.

## Changes
- update `SignedTransaction` to contain `Any` for transaction instead of
`UnsignedTransaction`

## Testing
unit test

## Breaking Changelist
- the `SignedTransaction` type is changed
- tx hashes are no longer the same

## Related Issues

closes #310
  • Loading branch information
noot authored May 10, 2024
1 parent 55f16b9 commit 171c765
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 39 deletions.

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

113 changes: 105 additions & 8 deletions crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ use ed25519_consensus::{
SigningKey,
VerificationKey,
};
use prost::Message as _;
use prost::{
Message as _,
Name as _,
};

use super::raw;

Expand Down Expand Up @@ -68,6 +71,7 @@ pub struct SignedTransaction {
signature: Signature,
verification_key: VerificationKey,
transaction: UnsignedTransaction,
transaction_bytes: bytes::Bytes,
}

impl SignedTransaction {
Expand All @@ -90,12 +94,16 @@ impl SignedTransaction {
let Self {
signature,
verification_key,
transaction,
transaction_bytes,
..
} = self;
raw::SignedTransaction {
signature: signature.to_bytes().to_vec(),
public_key: verification_key.to_bytes().to_vec(),
transaction: Some(transaction.into_raw()),
transaction: Some(pbjson_types::Any {
type_url: raw::UnsignedTransaction::type_url(),
value: transaction_bytes,
}),
}
}

Expand All @@ -104,12 +112,16 @@ impl SignedTransaction {
let Self {
signature,
verification_key,
transaction,
transaction_bytes,
..
} = self;
raw::SignedTransaction {
signature: signature.to_bytes().to_vec(),
public_key: verification_key.to_bytes().to_vec(),
transaction: Some(transaction.to_raw()),
transaction: Some(pbjson_types::Any {
type_url: raw::UnsignedTransaction::type_url(),
value: transaction_bytes.clone(),
}),
}
}

Expand All @@ -135,16 +147,17 @@ impl SignedTransaction {
let Some(transaction) = transaction else {
return Err(SignedTransactionError::unset_transaction());
};
let bytes = transaction.encode_to_vec();
let bytes = transaction.value.clone();
verification_key
.verify(&signature, &bytes)
.map_err(SignedTransactionError::verification)?;
let transaction = UnsignedTransaction::try_from_raw(transaction)
let transaction = UnsignedTransaction::try_from_any(transaction)
.map_err(SignedTransactionError::transaction)?;
Ok(Self {
signature,
verification_key,
transaction,
transaction_bytes: bytes,
})
}

Expand All @@ -155,6 +168,7 @@ impl SignedTransaction {
signature,
verification_key,
transaction,
..
} = self;
SignedTransactionParts {
signature,
Expand Down Expand Up @@ -211,6 +225,7 @@ impl UnsignedTransaction {
signature,
verification_key,
transaction: self,
transaction_bytes: bytes.into(),
}
}

Expand All @@ -226,6 +241,15 @@ impl UnsignedTransaction {
}
}

#[must_use]
pub fn into_any(self) -> pbjson_types::Any {
let raw = self.into_raw();
pbjson_types::Any {
type_url: raw::UnsignedTransaction::type_url(),
value: raw.encode_to_vec().into(),
}
}

pub fn to_raw(&self) -> raw::UnsignedTransaction {
let Self {
actions,
Expand All @@ -239,6 +263,11 @@ impl UnsignedTransaction {
}
}

#[must_use]
pub fn to_any(&self) -> pbjson_types::Any {
self.clone().into_any()
}

/// Attempt to convert from a raw, unchecked protobuf [`raw::UnsignedTransaction`].
///
/// # Errors
Expand All @@ -265,6 +294,22 @@ impl UnsignedTransaction {
params,
})
}

/// Attempt to convert from a protobuf [`pbjson_types::Any`].
///
/// # Errors
///
/// - if the type URL is not the expected type URL
/// - if the bytes in the [`Any`] do not decode to an [`UnsignedTransaction`]
pub fn try_from_any(any: pbjson_types::Any) -> Result<Self, UnsignedTransactionError> {
if any.type_url != raw::UnsignedTransaction::type_url() {
return Err(UnsignedTransactionError::invalid_type_url(any.type_url));
}

let raw = raw::UnsignedTransaction::decode(any.value)
.map_err(UnsignedTransactionError::decode_any)?;
Self::try_from_raw(raw)
}
}

#[derive(Debug, thiserror::Error)]
Expand All @@ -279,6 +324,16 @@ impl UnsignedTransactionError {
fn unset_params() -> Self {
Self(UnsignedTransactionErrorKind::UnsetParams())
}

fn invalid_type_url(got: String) -> Self {
Self(UnsignedTransactionErrorKind::InvalidTypeUrl {
got,
})
}

fn decode_any(inner: prost::DecodeError) -> Self {
Self(UnsignedTransactionErrorKind::DecodeAny(inner))
}
}

#[derive(Debug, thiserror::Error)]
Expand All @@ -287,6 +342,17 @@ enum UnsignedTransactionErrorKind {
Action(#[source] action::ActionError),
#[error("`params` field is unset")]
UnsetParams(),
#[error(
"encountered invalid type URL when converting from `google.protobuf.Any`; got `{got}`, \
expected `{}`",
raw::UnsignedTransaction::type_url()
)]
InvalidTypeUrl { got: String },
#[error(
"failed to decode `google.protobuf.Any` to `{}`",
raw::UnsignedTransaction::type_url()
)]
DecodeAny(#[source] prost::DecodeError),
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -367,9 +433,40 @@ mod test {
let tx = SignedTransaction {
signature,
verification_key,
transaction: unsigned,
transaction: unsigned.clone(),
transaction_bytes: unsigned.to_raw().encode_to_vec().into(),
};

insta::assert_json_snapshot!(tx.sha256_of_proto_encoding());
}

#[test]
fn signed_transaction_verification_roundtrip() {
let signing_key = SigningKey::from([
213, 191, 74, 63, 204, 231, 23, 176, 56, 139, 204, 39, 73, 235, 193, 72, 173, 153, 105,
178, 63, 69, 238, 27, 96, 95, 213, 135, 120, 87, 106, 196,
]);

let transfer = TransferAction {
to: Address::from([0; 20]),
amount: 0,
asset_id: default_native_asset_id(),
fee_asset_id: default_native_asset_id(),
};

let params = TransactionParams {
nonce: 1,
chain_id: "test-1".to_string(),
};
let unsigned = UnsignedTransaction {
actions: vec![transfer.into()],
params,
};

let signed_tx = unsigned.into_signed(&signing_key);
let raw = signed_tx.to_raw();

// `try_from_raw` verifies the signature
SignedTransaction::try_from_raw(raw).unwrap();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,36 +3,36 @@ source: crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs
expression: tx.sha256_of_proto_encoding()
---
[
45,
183,
116,
196,
217,
243,
23,
192,
66,
217,
1,
108,
44,
224,
168,
98,
240,
215,
197,
83,
178,
71,
115,
206,
104,
180,
159,
248,
74,
224,
209,
176,
241,
29,
14,
237,
215,
243,
132,
7,
196,
102,
232,
226,
192,
10,
114,
218,
129,
58,
199
197,
41,
11,
211,
188,
188,
49,
43,
168,
198
]
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package astria.protocol.transactions.v1alpha1;
import "astria/primitive/v1/types.proto";
import "astria_vendored/penumbra/core/component/ibc/v1/ibc.proto";
import "astria_vendored/tendermint/abci/types.proto";
import "google/protobuf/any.proto";

// `SignedTransaction` is a transaction that has
// been signed by the given public key.
Expand All @@ -13,7 +14,7 @@ import "astria_vendored/tendermint/abci/types.proto";
message SignedTransaction {
bytes signature = 1;
bytes public_key = 2;
UnsignedTransaction transaction = 3;
google.protobuf.Any transaction = 3;
}

// `UnsignedTransaction` is a transaction that does
Expand Down

0 comments on commit 171c765

Please sign in to comment.