Expose ATC transactions field with a getter method

[barnjamin]

Problem

Right now the ATC holds an array of TransactionWithSigner objects that is private

https://github.com/algorand/js-algorand-sdk/blob/develop/src/composer.ts#L120

This means to get access to the transactions, the developer must call buildGroup which changes the state of the ATC.

It is possible to clone the ATC prior to calling buildGroup so that the state of the original ATC is unchanged but this is not obvious.

Solution

Add a getter method to allow access to the transactions constructed so far

[jasonpaulos]

I think it's in our best interests to keep all implementation fields private. This prevents users from modifying any internal state in unexpected ways, and it makes maintenance and future code changes easier for us.

However, if you wanted to add a getter method to access individual transactions, that's probably reasonable.

[robdmoore]

Ben raised this after I provided some feedback to him about how ATC has a bunch of useful logic, but it's not very flexible for extension, hiding most of it away.

Agreed making protected isn't right, but I'd like to request the following two changes which would significantly improve the flexibility / extensibility:

• Return the transaction(s) / method call(s) that a call to addMethodCall creates (it may add multiple transactions, if they aren't returned then you have to do a .clone().buildGroup() to get the transactions every time you call it which is tedious and inefficient)
• Return the PendingTransactionInfo from execute (otherwise if you want that object you have to issue an additional algod call to get that data)

[jasonpaulos]

Thanks for that insight @robdmoore. I just have a few questions:

• Return the transaction(s) / method call(s) that a call to addMethodCall creates (it may add multiple transactions, if they aren't returned then you have to do a .clone().buildGroup() to get the transactions every time you call it which is tedious and inefficient)

This seems like a reasonable request. Can I ask why you're interested in seeing the transactions produced by addMethodCall though?

• Return the PendingTransactionInfo from execute (otherwise if you want that object you have to issue an additional algod call to get that data)

We currently do return the PendingTransactionInfo for all method calls from execute. They're accessible in ABIResult.txInfo. Is this what you're looking for, or do you also want PendingTransactionInfo for non-method calls?

[robdmoore]

Apologies for the delay in replying.

I want to see the transactions from addMethodCall because I have a wrapper that provides specific functionality that results in a call to addMethodCall and one of the return values from this method is the list of transaction(s) it generated. As an escape hatch and to allow for more advanced chaining, this method optionally takes an Atomic Transaction Composer so I can't just grab every single method in it since it may already have some methods.

Here's some relevant lines with one example: https://github.com/algorandfoundation/algokit-utils-ts/blob/main/src/app-deploy.ts#L198 and https://github.com/algorandfoundation/algokit-utils-ts/blob/main/src/app-deploy.ts#L234.

Yes - PendingTransactionInfo is what we want, I didn't realise it was available in the ABIResult. I do also want it for non-ABI calls too for consistency. Currently, I need to reissue HTTP requests to get this info per: https://github.com/algorandfoundation/algokit-utils-ts/blob/main/src/transaction.ts#L182-L187

https://github.com/algorandfoundation/algokit-utils-ts/blob/main/src/transaction.ts#L147

[jasonpaulos]

I agree it definitely makes sense to do these things:

• Add a method to the AtomicTransactionComposer to get individual unsigned transactions. This is a read-only operation, it can be used immediately after adding a transaction to the composer, and it doesn't change the status at all.
• Change the return type of AtomicTransactionComposer.addMethodCall from void to Transaction. The returned value would be the app call transaction created by addMethodCall.

About returning pending transaction info for non-method calls from AtomicTransactionComposer.execute: it isn't really any more efficient for the composer to do inside of its execute method than it is for the caller to do this. That's because a separate GET request is necessary for each transaction in the group, and right now the composer only GETs method call transactions. So there is no argument for efficiency here (in fact it'd probably be less efficient if we started doing this and callers didn't care about the results), but I could see some convenience/consistency benefits to this.