Add traits for a leaner transaction creation API

[manuelmauro]

Addin traits like this:

pub trait Pay {
    fn pay(&self, to: &Account, amount: MicroAlgos) -> TransactionType;
}

impl Pay for Account {
    fn pay(&self, to: &Account, amount: MicroAlgos) -> TransactionType {
        TransactionType::Payment(crate::transaction::Payment {
            sender: self.address,
            receiver: to.address,
            amount: amount,
            close_remainder_to: None,
        })
    }
}

we could have a more intuitive process when building transactions. For instance, this is how our payment.rs example would look like:

async fn main() -> Result<(), Box<dyn Error>> {
    // load variables in .env
    dotenv().ok();

    let algod = Algod::new(&env::var("ALGOD_URL")?, &env::var("ALGOD_TOKEN")?)?;
    let alice = Account::from_mnemonic("fire enlist diesel stamp nuclear chunk student stumble call snow flock brush example slab guide choice option recall south kangaroo hundred matrix school above zero")?;
    let bob = Account::from_mnemonic("since during average anxiety protect cherry club long lawsuit loan expand embark forum theory winter park twenty ball kangaroo cram burst board host ability left")?;

    let payment = alice.pay(&bob, MicroAlgos(123_456));
    let params = algod.suggested_transaction_params().await?;

    let txn = TxnBuilder::with(&params, payment).build()?;

    let sign_response = alice.sign_transaction(txn)?;

    let send_response = algod.broadcast_signed_transaction(&sign_response).await;
    println!("response: {:?}", send_response);

    Ok(())
}

Notice that the new trait will need to be in scope for this to work. Maybe this would require the creation of a prelude?

[ivnsch]

This doesn't work, because the accounts are usually not known when creating the transaction. It could be added to Address, but it would be confusing, because the signature suggest that you're paying, but you're just creating a transaction that still has to be signed and submitted. I think that the transaction builder api makes this clearer.

[manuelmauro]

Correct, Address. Concerning the signature, I suppose that would be confusing only to someone who is new to the transaction lifecycle.

With the current solution, I find ordered params and build invocations clumsy.

let t = TxnBuilder::with(
        &params,
        Pay::new(from.address(), to.address(), MicroAlgos(123_456)).build(),
    )
    .build()?;

[ivnsch]

Well, you mentioned intuitiveness.

For a fair comparison:

let txn = TxnBuilder::with(
    &params, 
    alice.pay(&bob, MicroAlgos(123_456))
).build()?;

vs.

let txn = TxnBuilder::with(
    &params,
    Pay::new(alice, bob, MicroAlgos(123_456)).build(),
).build()?;

Not a fan of the second (current) one, but the first one has something more awkward. The supposedly "nice" api is still embedded in the low-level builder. Further, you removed the builder from the transaction types, so we'd have to parametrize the "nice" api with potentially a lot of not used optionals.

And honestly, I find the recurrent discussion around the builder API misplaced currently, given that we're standing on shaky foundations: There's very little test coverage, we've not feature parity with the official SDKs, no process to sync with the node's APIs, etc. These cosmetic discussions have their place, but currently are taking focus away from urgent issues.

[ivnsch]

I find the recurrent discussion around the builder API misplaced

For the record, this had some additional context that was not mentioned here (mainly a conversation in which we seemed to agree about next steps). We talked about this now, and I want to add that I by no means want to discourage from submitting ideas, whatever they might be.

I'll be closing this, because in its current presentation it seems not something we want to pursue. Feel free to reopen or create a new issue if it's evolved or there's need for further discussion.