Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't specify arbitrary foreign accounts for method call #732

Closed
jeron-diovis opened this issue Jan 10, 2023 · 4 comments
Closed

Can't specify arbitrary foreign accounts for method call #732

jeron-diovis opened this issue Jan 10, 2023 · 4 comments
Labels
new-bug Bug report that needs triage

Comments

@jeron-diovis
Copy link

In a simplified way, my contract has following logic:

class Demo(Application):
    addressee = ApplicationStateValue(stack_type=TealType.bytes)

    @decorators.create
    def create(self, addressee: abi.Address):
        return Seq(
            self.addressee.set(addressee.get())
        )

    @decorators.external
    def notify(self):
        return Seq(
            InnerTxnBuilder.Execute({
                TxnField.type_enum: TxnType.Payment,
                TxnField.amount: Int(0),
                TxnField.note: Bytes("some notification"),
                TxnField.receiver: self.addressee.get(),
            })
        )

To make it work, addressee value must be explicitly submitted in accounts application array.

In python it works fine:

app = ApplicationClient(app=Demo(), app_id=..., ...)
app.call(Demo.notify, accounts=["whatever"])

In JS it doesn't:

import { Demo } from './beaker-generate/demo_client'
const app = new Demo({ appId: ..., ... })

app.notify({
  appAccounts: [ "whatever" ]
})

// still reports `logic eval error: invalid Account reference` error.

Which is because in JS SDK accounts field is composed internally, based on method args only. While appAccounts field is not used at all, despite it's declared as a valid TransactionOverrides parameter.

I can construct txn manually, using makeApplicationCallTxn directly.
But can't use ATC – and thus can't use beaker-ts ApplicationClient - with all the handy stuff it provides.

Is this a bug or there is some intent here?

@jeron-diovis jeron-diovis added the new-bug Bug report that needs triage label Jan 10, 2023
@jeron-diovis
Copy link
Author

jeron-diovis commented Jan 10, 2023

Looks like there is an intent.
https://forum.algorand.org/t/javascript-sdk-atomictransactioncomposer-addmethodcall-missing-accounts-argument/7715

This will lead to a bloated interface (my app has 3 addresses in state, used in many methods), but, well, if rules say so, I'll rewrite it.

But, I'm curious – why py-SDK allows it then? It's really misleading, when same SDK behaves differently in different languages.
And appAccounts shouldn't be an allowed parameter for TransactionOverrides in ApplicationClient methods – although it's an issue for beaker-ts repo.

@barnjamin
Copy link
Contributor

Once this is merged #725 you'll be able to do this.

Alternatively you could specify that the Address is of type Account instead, this would make the sdk machinery work as it is right now, but that might conflict with some already deployed contract?

@jeron-diovis
Copy link
Author

@barnjamin My app is not released yet, so any experiments are ok. But I don't see how changing param type can help here, could you please explain it a bit?

And, as #725 is merged yesterday, when it is expected to be released?

@barnjamin
Copy link
Contributor

The pr allows you to specify arbitrary accounts that are not included in the method signature of the ABI method. Once its tagged and released, beaker-ts can be updated to include it you'll be able to specify the accounts.

Until then an alternative for this specific instance would be for you to change the method signature of notify to expect an abi.Account argument and pass the account as a normal argument when calling it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-bug Bug report that needs triage
Projects
None yet
Development

No branches or pull requests

2 participants