Integrate kairos-tx with smart contract events

[koxu1996]

As discussed during the latest Smart Contract deep dive, we should include details of L2 transaction in deposit event:

• payload - SigningPayload from kairos-tx,
• public key,
• signature.

Furthermore, smart contract must check that given transaction details are correct, i.e.:

• Given public key is matching with contract caller (depositor).
• Signature is valid for payload.
• Payload details are matching with deposit amount.

That would allow to us to fully trust deposit events, without additional interactions with L1.

[koxu1996]

The first idea was to integrate casper_event_standard with kairos-tx - add #[derive(Event)] and emit transaction as the whole deposit event. However, that direct/deep integration is overkill, let me explain why.

Integrating CES with kairos-tx would begin with:

diff --git a/kairos-tx/Cargo.toml b/kairos-tx/Cargo.toml
index a2838c7..dac3a89 100644
--- a/kairos-tx/Cargo.toml
+++ b/kairos-tx/Cargo.toml
@@ -9,7 +9,9 @@ license.workspace = true
 [dependencies]
 num-traits = "0.2"
 rasn = { version = "0.12", default-features = false, features = ["macros"] }
+casper-event-standard = { version = "0.5.0", optional = true }
 
 [features]
 default = ["std"]
 std = []
+ces = ["casper-event-standard"]
diff --git a/kairos-tx/src/asn.rs b/kairos-tx/src/asn.rs
index 383c720..8a67ca8 100644
--- a/kairos-tx/src/asn.rs
+++ b/kairos-tx/src/asn.rs
@@ -6,6 +6,7 @@ use crate::error::TxError;
 // Expose types for the public API.
 pub use rasn::types::{Integer, OctetString};
 
+use casper_event_standard::Event;
 use num_traits::cast::ToPrimitive;
 use rasn::types::AsnType;
 use rasn::{Decode, Encode};
@@ -59,6 +60,7 @@ impl TryFrom<Nonce> for u64 {
 }
 
 #[derive(AsnType, Encode, Decode, Debug)]
+#[cfg_attr(feature = "ces", derive(Event))]
 #[non_exhaustive]
 pub struct SigningPayload {
     pub nonce: Nonce,

So far so good, but we have to derive Event for all nested fields too. This doesn't work for newtypes - Nonce, Amount, PublicKey structs, e.g.:

error: Expected a struct with named fields.
  --> kairos-tx/src/asn.rs:46:1
   |
46 | / #[rasn(delegate)]
47 | | pub struct Nonce(pub(crate) Integer);
   | |_____________________________________^

Macro is expecting named fields, so derive doesn't work for enum too:

error: Expected a struct with named fields.
  --> kairos-tx/src/asn.rs:71:1
   |
71 | / #[rasn(choice)]
72 | | #[non_exhaustive]
73 | | pub enum TransactionBody {
74 | |     #[rasn(tag(0))]
...  |
79 | |     Withdrawal(Withdrawal),
80 | | }
   | |_^

To solve this we could implement 4 traits for each type incompatible with macro:

• casper_event_standard::casper_types::CLTyped
• casper_event_standard::casper_types::bytesrepr::ToBytes
• casper_event_standard::casper_types::bytesrepr::FromBytes
• casper_event_standard::EventInstance

At this point I am sure we should go with better and easier idea: include transaction details in the smart contract event as the bytes. I am going to submit PR for that, cc @jonas089.

[jonas089]

@koxu1996 So we just serialize the event metadata to Bytes before we emit it? I always preferred that so I agree :).

[jonas089]

But then we aren't really using ASN, one would have to construct the ASN format from standardized metadata, correct?

[koxu1996]

@jonas089 serialized transaction data - which is constructed from ASN.1 - would be included as bytes in Deposit event, for example:

#[derive(Event)]
pub struct Deposit {
    // ...
    pub tx: Vec<u8>,
}

Those tx bytes will be fully constructed by the user, however we need to parse and validate it in the smart contract.

[jonas089]

@jonas089 serialized transaction data - which is constructed from ASN.1 - would be included as bytes in Deposit event, for example:

#[derive(Event)]
pub struct Deposit {
    // ...
    pub tx: Vec<u8>,
}

Those tx bytes will be fully constructed by the user, however we need to parse and validate it in the smart contract.

I see, so it is a nested struct for serialized ASN data. So we have 2 options:

1. deserialize in the smart contract and emit the serialized input
2. serialize in the smart contract

and you prefer 1.?

[jonas089]

@jonas089 serialized transaction data - which is constructed from ASN.1 - would be included as bytes in Deposit event, for example:

#[derive(Event)]
pub struct Deposit {
    // ...
    pub tx: Vec<u8>,
}

Those tx bytes will be fully constructed by the user, however we need to parse and validate it in the smart contract.

I see, so it is a nested struct for serialized ASN data. So we have 2 options:

1. deserialize in the smart contract and emit the serialized input
2. serialize in the smart contract

and you prefer 1.?

Because the contract needs to read and validate the transaction data as you mentioned before, so does it deserialize the ASN struct?

[koxu1996]

I see, so it is a nested struct for serialized ASN data. So we have 2 options:

1. deserialize in the smart contract and emit the serialized input
2. serialize in the smart contract

@jonas089 Smart contract must deserialize transaction data to confirm that given details are correct, e.g. L1 transferred amount is equal to amount specified inside tx. Data already comes as serialized directly from user, so there is no serialization in the smart contract (unless you want to double check that bytes are exactly the same after full serialization roundtrip).

[jonas089]

I'm just wondering how expensive deserialization is / if it is more expensive than serialization

[jonas089]

Maybe a host function for deserialization could make sense in the future, but for now we can just do it in the contract and adjust the chainspec if necessary.

[koxu1996]

I don't think this will be ever a bottleneck. Btw., I just tested kairos-tx with full transaction format, to test deserialization cost for deposit tx - it is less than 0.05 CSPR:

[koxu1996]

While using kairos-tx I stumbled across compatibility issue with wasm32-unknown-unknown target, caused by macro library rasn-derive casper_types relying on getrandom.

Compilation error:

   Compiling rasn-derive v0.12.5 (/tmp/kairos-contracts/demo-contract/contract/rasn-derive)
error: the wasm*-unknown-unknown targets are not supported by default, you may need to enable the "js" feature. For more information see: https://docs.rs/getrandom/#webassembly-support
   --> /home/andrew/.cargo/registry/src/index.crates.io-6f17d22bba15001f/getrandom-0.2.14/src/lib.rs:352:9
    |
352 | /         compile_error!("the wasm*-unknown-unknown targets are not supported by \
353 | |                         default, you may need to enable the \"js\" feature. \
354 | |                         For more information see: \
355 | |                         https://docs.rs/getrandom/#webassembly-support");
    | |________________________________________________________________________^

error[E0433]: failed to resolve: use of undeclared crate or module `imp`
   --> /home/andrew/.cargo/registry/src/index.crates.io-6f17d22bba15001f/getrandom-0.2.14/src/lib.rs:408:9
    |
408 |         imp::getrandom_inner(dest)?;
    |         ^^^ use of undeclared crate or module `imp`

Underneath it uses random UUID for syn::Lifetime, however it could be replaced with atomic counter. Working on patch ⚙️.

---

Update: This is not issue with rasn, but with usage of Casper types library. When std feature is enabled, it will use random for generating keys. This already got fixed with #78 - submitted to solve the issue described in the next comment.

[koxu1996]

Using kairos-crypto is also not compatible with wasm32-unknown-unknown target, which is caused by enabled filesystem feature in casper_types dependency.

Compilation error:

Compiling casper-types v4.0.1
error[E0433]: failed to resolve: could not find `unix` in `os`
 --> /home/andrew/.cargo/registry/src/index.crates.io-6f17d22bba15001f/casper-types-4.0.1/src/file_utils.rs:6:9
  |
6 |     os::unix::fs::OpenOptionsExt,
  |         ^^^^ could not find `unix` in `os`

error[E0599]: no method named `mode` found for mutable reference `&mut OpenOptions` in the current scope
  --> /home/andrew/.cargo/registry/src/index.crates.io-6f17d22bba15001f/casper-types-4.0.1/src/file_utils.rs:70:10
   |
67 | /     fs::OpenOptions::new()
68 | |         .write(true)
69 | |         .create(true)
70 | |         .mode(0o600)
   | |         -^^^^ method not found in `&mut OpenOptions`
   | |_________|

Fortunately fixing this is simple - I will hide filesystem functions in kairos-crypto behind feature flag.

---

Update: fix submitted #78.

[marijanp]

Not a big fan of serializing/deserializing. I think the implementation the traits for newtype types is straightforward. With the enum case I don't fully understand why you want to derive Event for TransactionBody, I thought we only want to include all information regarding deposits.

[koxu1996]

@marijanp There is no serialization involved. Deserialization will be used for L2 transaction validation - you can see prototype here.

With the enum case I don't fully understand why you want to derive Event for TransactionBody

In the first comment I explained why we do NOT want to derive Event for transaction structs, and lack of support for enum was just one of the examples. The final conclusion was:

At this point I am sure we should go with better and easier idea: include transaction details in the smart contract event as the bytes

So deposit event will look like this:

#[derive(Event)]
pub struct Deposit {
    // ...
    pub tx: Vec<u8>,
}

Where tx is serialized transaction data received from the user.

[marijanp]

What I mean is why do you even consider to add the Event trait for the Transaction type. I understand that we do not want to derive it for Transaction because the macro doesn't support enum types. But if we would just focus on the Deposit struct and derive Event for it, which is supported because it's a struct. That would then also make more sense as you cannot or we are not going to emit Withdraw or Transfer events.

Maybe it's better to duplicate the nonce field in each Deposit, Transfer and Withdraw type, getting rid of the TransactionBody enum. Also note that we are basically going to ignore the nonce for deposits anyways.

I'm suggesting rethinking the whole type hierarchy that we have, and then making our lives easier by just being able to derive Event and not having to deal with deserialization on-chain, and we benefit from more type safety.