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

bug(TXE): Error messages in nested calls don't bubble up #7362

Closed
Tracked by #2783
LHerskind opened this issue Jul 5, 2024 · 0 comments · Fixed by #7730
Closed
Tracked by #2783

bug(TXE): Error messages in nested calls don't bubble up #7362

LHerskind opened this issue Jul 5, 2024 · 0 comments · Fixed by #7730
Assignees

Comments

@LHerskind
Copy link
Contributor

LHerskind commented Jul 5, 2024

Currently if a failure happens in a nested call (foreign call) the error message will not be bubbling up.

For example, the following test will fail, but with an empty message instead of the expected.

#[test(should_fail_with="Balance too low")]
unconstrained fn transfer_private_failure_more_than_balance() {
    // Setup without account contracts. We are not using authwits here, so dummy accounts are enough
    let (env, token_contract_address, _, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false);
    // Transfer tokens
    let transfer_amount = mint_amount + 1;
    let transfer_private_call_interface = Token::at(token_contract_address).transfer(recipient, transfer_amount);
    env.call_private_void(transfer_private_call_interface);
}

Notably, if run just as a test #[test], you will get the following.

Failed calling external resolver. RPC error response: RpcError { code: -32000, message: "Assertion failed: Balance too low", data: None } 

So ideally it should be passing the first test.

@github-project-automation github-project-automation bot moved this to Todo in A3 Jul 5, 2024
@LHerskind LHerskind self-assigned this Jul 5, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in A3 Aug 6, 2024
AztecBot pushed a commit to AztecProtocol/aztec-nr that referenced this issue Aug 7, 2024
Replaces AztecProtocol/aztec-packages#7271
Closes AztecProtocol/aztec-packages#7142
Closes AztecProtocol/aztec-packages#7362

This is quite similar to the implementation in #7271: `transfer`
consumes two notes, and if their amount is insufficient for the desired
transfer it calls a second internal function which recursively consumes
8 notes per iteration until either the amount is reached, or no more
notes are found. If the total amount consumed exceeds the transfer
amount, a change note is created.

This results in a much smaller transfer function for the scenario in
which two notes suffice, since we're using a `limit` value of 2 and
therefore only doing two iterations of note constraining (the expensive
part). Two notes is a good amount as it provides a way for change notes
to be consumed in follow-up calls.

The recursive call has a much lower gate count, since it doesn't need to
fetch keys, create notes, emit events, etc., and so we can afford to
read more notes here while still resulting in a relatively small
circuit.

I created a new e2e test in which the number of notes and their amounts
are quite controlled in order to properly test this. The tests are
unfortunately currently interdependent, but given the relative
simplicity of the test suite it should be hopefully simple to maintain
and expand, and maybe eventually fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant