WIP: Implement TransactionSet derivation

[slowli]

Seeing the struggles with #532, I've decided to try implementing it with procedural macros:

#[derive(Clone, TransactionSet)]
pub enum Transactions {
    CreateWallet(CreateWallet),
    Transfer(Transfer),
}

I'd be eager to expand the approach to the messages themselves as well, but it is hindered by uncertainties with their serialization.

[slowli]

I'm afraid this is the most is possible to do right now with derivation. It could be possible to force transactions within TransactionSet to ignore their message_ids in tx_from_raw and Deserialize implementation, but it would break transaction creation. (The underlying problem is that transactions are messages.)

[alekseysidorov]

I wonder can we implement derivation on top of the rust enums with the some attributes?

#[derive(TransactionSet)]
#[transaction_set(service_id = "10")]
enum MyTransactions {
    TxFirst {
        from: PublicKey,
        data: Vec<u8>,
    },
    TxSecond {
        from: PublicKey,
        data: String
    }
}

There is one limitation: we can't use constants in the attributes.

[slowli]

@alekseysidorov That's an interesting idea: to automatically create message types for variants. That is, the code above would generate

pub struct TxFirst {
    raw: RawMessage,
}

impl TxFirst {
    // ...
}

// other auto-gen implementations currently generated by `messages!`

May be worth a try (and if proc macros are processed before "ordinary" ones, it would be easy to prototype, too - just read variants and shove them into a messages! macro!). There is an obvious problem with this approach, though: in the current form, message definitions are un-grammatic; e.g., field definition from: &PublicKey lacks a lifetime specifier. In order to be Rust-y, we need to write something like

enum Transactions<'a> {
    TxFirst {
        pub from: &'a PublicKey,
        pub data: &'a [u8],
    },
    TxSecond {
        pub from: &'a PublicKey,
        pub data: &'a str,
    }
}

(in which case transactions become non-owning thin wrappers around byte buffers - not quite sure it will bode well with the current paradigm of transaction processing, tbh), or somehow determine ownership and map types correspondingly when processing the macro. Do I understand correctly that you propose the second approach? Or is your proposition will be most valid after changing transaction serialization principles?

There is one limitation: we can't use constants in the attributes.

What do you mean? I think we can make at least this to work:

pub const SERVICE_ID: u16 = 10;

#[derive(TransactionSet)]
#[exonum(service_id = "SERVICE_ID")]
enum MyTransactions { /* ... */ }

just as serde somehow manages to convert strings supplied to #[serde(serialize_with = "...")]-like attributes to method invocations.

[alekseysidorov]

Now we can invoke messages macro under the hood and do not to use resulting Transactions directly.
Instead of this we can generate tx_from_raw method that will return Box<Transaction>.
But if we want to use serde, this macros becomes completely human friendly.

[stanislav-tkach]

Looks promising, but I'm a little bit concerned about one aspect of this change. transactions! macro with its "foreign" syntax (and all non-procedural macros disadvantages) makes accent on declaring a set of the specific entities. With procedural macros we have a common structure with some auxiliary properties: just like with serialization.

[slowli]

@DarkEld3r

makes accent on declaring a set of the specific entities.

But it does declare a separate type for the TransactionSet, right? And if what @alekseysidorov proposed is workable, then it would be possible to write something like

let tx = Transactions::Transfer {
    public_key: &my_pubkey,
    to: &some_other_pubkey,
    amount: 100,
    seed: 1,
};
let tx: Box<Transaction> = tx
    .sign(&my_secret_key)
    .into();

This is, imo, quite an improvement over the current syntax of transaction creation.

[stanislav-tkach]

As for me, transactions first of all isn't just some data type. It is exonum transaction: specific logic plus data. 🙃
Still, I'm not against these changes, I would love to see their impact on our code-base.

[stanislav-tkach]

One more thought. Can we (theoretically and in the future) expand transactions! macro, so that verify and execute methods will be defined inside?

[alekseysidorov]

interesting can we generate json spec for light client in the transaction documentation?

[slowli]

Here is a very rough sketch of how codegen might look like if we take my idea about non-owning transactions (the user code is at the bottom). I'll give it a try in the next couple of days. The only worry is that it can become obsolete if we choose owning transactions when serialization is changed.

@alekseysidorov

interesting can we generate json spec for light client in the transaction documentation?

Seems like it, but it can probably wait 😃

[slowli]

I've managed to make borrowed transactions to work; see this file for an example of what it looks like. Couple of comments:

1. Changes are backward-compatible (seemingly; needs more testing)

2. Transaction payload and wire format are completely separated. If you look at Message derivation code, it does not need any info about the payload inner structure, only the path to its type. (This is accomplished by effectively splitting Message into 3 traits for checking, reading, and writing messages, and by making ExonumJsonDeserialize generic on its output.) Thus, ideally I would like to move it to core:

// in `exonum::messages`
pub struct Wrapper<T> {
    raw: RawMessage,
    _marker: ::std::marker::PhantomData<T>,
}

// implementations of `Message`, `ExonumJson`, etc. 

However, this currently doesn't work, because T (i.e., the payload type) may contain a lifetime parameter, which makes implementing traits without a lifetime (e.g., ExonumJson) impossible (or at least I haven't figured out how to do it).

[vitvakatu]

What is the reason behind adding new method? Let's pass field by reference in the original write

[vldm]

Can we close this and repeat work sometimes later? @DarkEld3r @slowli

[slowli]

Severely outdated.