Allow for Account.SignTxns(params T[] txns)

[bernhardrieder]

There's currently (in version 2.2.0 at least) only Acount.SignTxns(T[] txns) and it would be more convenient to have it with the params keyword like it's being used througout the SDK.

[bernhardrieder]

And perhaps also an option to allow for a custom group id with SignTxns? There's a place in the auction demo where 3 transactions are being sent with the same group id but with 2 different accounts.

[bernhardrieder]

On this note, is it somehow possible to use the params T[] for different types of transactions at once or is it intended to be used only with the same transaction type?

For example, you have a PaymentTxn and one AppCallTxn, then you have to call TransactionGroup.Of(payTxn.GetId(), appCallTxn.GetId()).GetId() instead of just TransactionGroup.Of(payTxn, appCallTxn).GetId(). That is because T requires to be ITransaction and IEquatable<T>.

I did a quick google search but I couldn't find anything about making this possible and my C# skills are to limited to figure something out on the get-go 😀

[jasonboukheir]

There is a simple way to do that, cast them to ITransaction interface:

public byte[] SignTxns(params ITransaction[] txns);

This causes allocations for the array and the interface wrapper.

There is another way using generics and type args that won't cause allocations, but there is a limit to the number of transactions supported (there's an upper limit of 16 transactions per group anyway)

public byte[] SignTxns<T0, T1, ..., TN>(T0 txn0, T1 txn1, ..., TN txnN)
    where T0 : ITransaction
    where T1 : ITransaction
    // ...
    where TN : ITransaction
{
    // create an array of transaction ids and add them one by one...
}

The same thing can be done for the TransactionGroup.Of, use a generic with variable number of arguments.

[jasonboukheir]

And perhaps also an option to allow for a custom group id with SignTxns? There's a place in the auction demo where 3 transactions are being sent with the same group id but with 2 different accounts.

The SignTxns should sign only the transactions that have Sender == Account.Address. At the moment you'd have to combine them:

var txns = new Transaction[3] { /* my txns */ };
var signedByAcct1 = acct1.SignTxns(txns);
var signedByAcct2 = acct2.SignTxns(txns);

var signedTxns = new SignedTxn[3];
for (var i = 0; i < txns.Length; i++)
{
  signedTxns[i] = signedByAcct1[i].Sig.Equals(default)
    ? signedByAcct2[i]
    : signedByAcct1[i];
}

Maybe to make this easier in the meantime, I could add SignedTxn.IsSigned property?

Ideal API?

If we can get a consistent interface across all IAccount, then we could do the following:

Account acct = /* my account */;
KmdAccount kmdAcct = /* kmd account */;
WalletConnectAccount walletConnectAcct = /* WalletConnect account */;

var signedTxnGroup = await Transaction.Atomic(assetOptInTxn, assetTransferTxn, payTxn)
  .SignBy(acct)
  .SignBy(kmdAcct)
  .SignBy(walletConnectAcct);

algod.SendTransaction(signedTxnGroup);

Other Account types would be things like LogicAccount and MultisigAccount.

If you need to share the TxnGroup across different clients, then you implement your own IAccount interface.

TransactionGroup.Of could be renamed to Transaction.Atomic to show it's an atomic transaction. I think that resembles the other constructor static methods as well (Transaction.Payment, Transaction.AppCall, etc.).

Version 3 is fast approaching 🤯

[bernhardrieder]

The SignTxns should sign only the transactions that have Sender == Account.Address.

So you basically have to always pass all the transactions to both signers and it'll only sign the ones the signer cares about. This works fine if you do it that way, but if you're not always passing all the transactions to the signers, then you'd end up with different transaction group id from what I can see.

That's how the func looks right now and it would just disregard whatever group you assigned beforehand if you did so.

public SignedTxn<T>[] SignTxns<T>(T[] txns) where T : ITransaction, IEquatable<T>
{
    TransactionId id = TransactionGroup.Of(txns).GetId();
    SignedTxn<T>[] array = new SignedTxn<T>[txns.Length];
    for (int i = 0; i < txns.Length; i++)
    {
        T txn = txns[i];
        txn.Group = id;
        array[i] = (txn.Sender.Equals(Address) ? SignTxn(txn) : new SignedTxn<T>
        {
            Txn = txn
        });
    }

    return array;
}

This might cause some troubles for some people and perhaps it'd be good to check if the group id is already assigned and not take care of the group id? But I like your ideal API example, it looks like a good approach!

[jasonboukheir]

This works fine if you do it that way, but if you're not always passing all the transactions to the signers, then you'd end up with different transaction group id from what I can see.

There's a security reason it's like this for WalletConnect. If you give the wallet a transaction that doesn't have the full group, and the wallet assumes that the group id is correct, then you can fool the wallet into believing you're going to send a transaction.

Account is a local signer, but it has this interface only because I'm trying to keep the API similar consistent across IAccount. Probably would be clearer if it was named SignTxnGroup instead of SignTxns. If you want to manually assign group id, it's easy to do that via SignTxn. It will not replace the existing group id:

// if you're concerned about allocations, you can use Linq faster: https://github.com/jackmott/LinqFaster
using System.Linq;

var signed = txns.Select(account.SignTxn).ToArray();

I could make it more visible that the group id should match the group in SignTxnGroup, with an error or a warning or something.

[bernhardrieder]

I see, that makes sense. Yea, warnings are always good! :)

[jasonboukheir]

@bernhardrieder hey there, just wanna say that I moved this issue into a project in the @CareBoo org. I'm planning this ticket for a Version 3 release using the Ideal API. You should be able to see the progress in that board.

I'll be using that board going forward for planning.

[jasonboukheir]

Blocked by #130 (kind of... there are alternatives, but if I can get the Transaction to be unmanaged it makes this one a lot easier)

No longer blocked.

[jasonboukheir]

duplicate of #131