feat: expose AtomicTransactionComposer.methodCalls #860
Conversation
| @@ -122,9 +122,11 @@ export class AtomicTransactionComposer { | |||
| /** The maximum size of an atomic transaction group. */ | |||
| static MAX_GROUP_SIZE: number = 16; | |||
|
|
|||
| /** A map of tranasction indexes to the respective ABI method being called. Used for properly decoding return values. */ | |||
| methodCalls: Map<number, ABIMethod> = new Map(); | |||
There was a problem hiding this comment.
I'd prefer you leave the field as it was, but introduce a new getter method like getMethodCalls which returns a copy of the map.
That helps with maintainability and prevents clients from unintentionally modifying the internal map
There was a problem hiding this comment.
The problem is that modifying the map is sometimes necessary. For example, if you have an ABI method call that takes in another ABI method call as an argument. The ATC does not currently support this, so you need to get the appl transaction and update methodCalls if you want the return values. I suppose we could add support for that, but it would be a larger change to the ATC.
There was a problem hiding this comment.
If that's the use case you're pursuing, then yes I think we need to talk about how the interface can support that. Directly modifying the internal map is not a stable solution
There was a problem hiding this comment.
I suppose we could also have a method setMethodCall(idx: number, method: ABIMethod) in addition to getMethodCalls
There was a problem hiding this comment.
And there can be sanity checks that it's an appl that doesn't already have a method set
Makes
AtomicTransactionComposer.methodCallspublic. This is useful for modifying/retrieving the method calls in an ATC externally.Specifically useful for algorandfoundation/algokit-utils-ts#234