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

Warn if callbacks are created outside of transactions #1347

Closed
berzanorg opened this issue Dec 23, 2023 · 1 comment · Fixed by #1430
Closed

Warn if callbacks are created outside of transactions #1347

berzanorg opened this issue Dec 23, 2023 · 1 comment · Fixed by #1430
Labels
error-messages Issues about making error messages better

Comments

@berzanorg
Copy link

Currently, when a callback is created outside of a transaction, it neither throws nor warns the developer by an Eslint rule.
There isn't a runtime error while doing so, but as the callback doesn't work as expected it breaks the logic.
And developers see a completely unrelated error message which is unrelated to callbacks because the logic is not working as expected.
I don't know if there is a use case for callbacks to be created outside of transactions.
If there is none, I recommend adding an Eslint rule to warn users when callbacks are created outside of transactions.
If anyone can tell me where and how can I contribute to official o1js Eslint plugin, then I'll gladly do it.

This doesn't work as expected. (as the callback is created outside)

const callback = Experimental.Callback.create(zkAppB, 'approveSend', [amount])

const tx = await Mina.transaction(feePayerAddress, () => {
    AccountUpdate.fundNewAccount(feePayerAddress)
    token.transfer(exchangeAddress, tokenAccount1, amount, callback)
})

But this works.

const tx = await Mina.transaction(feePayerAddress, () => {
    const callback = Experimental.Callback.create(zkAppB, 'approveSend', [amount])
    AccountUpdate.fundNewAccount(feePayerAddress)
    token.transfer(exchangeAddress, tokenAccount1, amount, callback)
})
@berzanorg berzanorg changed the title Warn if callbacks are used outside of transactions Warn if callbacks are created outside of transactions Dec 23, 2023
@mitschabaude mitschabaude added the error-messages Issues about making error messages better label Jan 19, 2024
@mitschabaude
Copy link
Member

Thanks for raising this. This could be fixed I think, by running the Callback lazily. But it would be just another instance of internal complication to support magical APIs, which I think we should stop doing.

I'm on the side that we should probably remove Callback. It's a trivial wrapper around running the callback method, and passing the account update that it created on to the other method. That's a much less magical way to think about it, and the behaviour of having to do it in your transaction is more obvious

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error-messages Issues about making error messages better
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants