Skip to content

Commit 0607b77

Browse files
authored
feat: restore txe public sim errors (#16051)
These had been removed due to me not investigating why there were gone. After looking into things I learned that the public processor only deals with contract code and not the full artifact, and as such is unable to interpret a reverted call. We therefore do this in the TXE oracle using the `enrich` functions, which do access the full artifact and convert the revert reason selector into a proper message. I also added error strings for contract tests that lacked them
1 parent 5297bba commit 0607b77

File tree

15 files changed

+67
-45
lines changed

15 files changed

+67
-45
lines changed

noir-projects/aztec-nr/aztec/src/state_vars/public_immutable/test.nr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ unconstrained fn initialize_uninitialized_other_tx() {
6060
});
6161
}
6262

63-
#[test(should_fail)]
63+
#[test(should_fail_with = "Duplicate")]
6464
unconstrained fn initialize_already_initialized_same_tx() {
6565
let env = TestEnvironment::_new();
6666

@@ -75,7 +75,7 @@ unconstrained fn initialize_already_initialized_same_tx() {
7575
});
7676
}
7777

78-
#[test(should_fail)]
78+
#[test(should_fail_with = "already present")]
7979
unconstrained fn initialize_already_initialized_other_tx() {
8080
let env = TestEnvironment::_new();
8181

noir-projects/noir-contracts/contracts/app/auth_contract/src/test.nr

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,22 @@ unconstrained fn authorized_initially_unset() {
2727
assert_eq(env.view_public(auth.get_authorized()), std::mem::zeroed());
2828
}
2929

30-
#[test(should_fail)]
30+
#[test(should_fail_with = "caller is not admin")]
3131
unconstrained fn non_admin_cannot_set_unauthorized() {
3232
let (env, auth_contract_address, _, _, other) = setup();
3333
let auth = Auth::at(auth_contract_address);
3434

3535
let _ = env.call_public(other, auth.set_authorized(other));
3636
}
3737

38+
#[test(should_fail_with = "caller is not authorized")]
39+
unconstrained fn non_authorized_cannot_do_authorized_thing() {
40+
let (env, auth_contract_address, _, _, other) = setup();
41+
let auth = Auth::at(auth_contract_address);
42+
43+
let _ = env.call_private(other, auth.do_private_authorized_thing());
44+
}
45+
3846
#[test]
3947
unconstrained fn admin_can_schedule_set_authorized() {
4048
let (env, auth_contract_address, admin, to_authorize, _) = setup();

noir-projects/noir-contracts/contracts/app/easy_private_voting_contract/src/test/first.nr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ unconstrained fn test_end_vote() {
3535
});
3636
}
3737

38-
#[test(should_fail)]
38+
#[test(should_fail_with = "Only admin can end votes")]
3939
unconstrained fn test_fail_end_vote_by_non_admin() {
4040
let (mut env, voting_contract_address, _) = utils::setup();
4141
let alice = env.create_light_account();
@@ -88,7 +88,7 @@ unconstrained fn test_cast_vote_with_separate_accounts() {
8888
});
8989
}
9090

91-
#[test(should_fail)]
91+
#[test(should_fail_with = "already exists")]
9292
unconstrained fn test_fail_vote_twice() {
9393
let (mut env, voting_contract_address, _) = utils::setup();
9494
let alice = env.create_light_account();

noir-projects/noir-contracts/contracts/app/nft_contract/src/test/access_control.nr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ unconstrained fn access_control() {
2626
assert(!env.view_public(nft.is_minter(new_admin)));
2727
}
2828

29-
#[test(should_fail)]
29+
#[test(should_fail_with = "caller is not an admin")]
3030
unconstrained fn set_admin_requires_admin_caller() {
3131
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough
3232
let (env, nft_contract_address, _, other) = utils::setup(/* with_account_contracts */ false);
@@ -36,7 +36,7 @@ unconstrained fn set_admin_requires_admin_caller() {
3636
let _ = env.call_public(other, nft.set_admin(other));
3737
}
3838

39-
#[test(should_fail)]
39+
#[test(should_fail_with = "caller is not an admin")]
4040
unconstrained fn set_minter_requires_admin_caller() {
4141
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough
4242
let (env, nft_contract_address, _, other) = utils::setup(/* with_account_contracts */ false);

noir-projects/noir-contracts/contracts/app/nft_contract/src/test/minting.nr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ unconstrained fn mint_success() {
1313
utils::assert_owns_public_nft(env, nft_contract_address, owner, token_id);
1414
}
1515

16-
#[test(should_fail)] // should_fail_with="NFT minted by non-minter"
16+
#[test(should_fail_with = "caller is not a minter")]
1717
unconstrained fn mint_as_non_minter_fails() {
1818
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough
1919
let (env, nft_contract_address, owner, other) = utils::setup(/* with_account_contracts */ false);
@@ -22,7 +22,7 @@ unconstrained fn mint_as_non_minter_fails() {
2222
let _ = env.call_public(other, NFT::at(nft_contract_address).mint(owner, token_id));
2323
}
2424

25-
#[test(should_fail)] // should_fail_with="NFT not minted"
25+
#[test(should_fail_with = "token already exists")]
2626
unconstrained fn mint_twice_fails() {
2727
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough
2828
let (env, nft_contract_address, owner, _) = utils::setup(/* with_account_contracts */ false);
@@ -39,7 +39,7 @@ unconstrained fn mint_twice_fails() {
3939
let _ = env.call_public(owner, NFT::at(nft_contract_address).mint(owner, token_id));
4040
}
4141

42-
#[test(should_fail)]
42+
#[test(should_fail_with = "zero token ID not supported")]
4343
unconstrained fn mint_id_0_fails() {
4444
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough
4545
let (env, nft_contract_address, owner, _) = utils::setup(/* with_account_contracts */ false);

noir-projects/noir-contracts/contracts/app/nft_contract/src/test/transfer_in_public.nr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ unconstrained fn transfer_in_public_on_behalf_of_other() {
5555
utils::assert_owns_public_nft(env, nft_contract_address, recipient, token_id);
5656
}
5757

58-
#[test(should_fail)] // should_fail_with = "Assertion failed: Invalid authwit nonce. When 'from' and 'msg_sender' are the same, 'authwit_nonce' must be zero"
58+
#[test(should_fail_with = "Assertion failed: Invalid authwit nonce. When 'from' and 'msg_sender' are the same, 'authwit_nonce' must be zero")]
5959
unconstrained fn transfer_in_public_failure_on_behalf_of_self_non_zero_nonce() {
6060
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough.
6161
// The authwit check is in the beginning so we don't need to waste time on minting the NFT and transferring
@@ -70,7 +70,7 @@ unconstrained fn transfer_in_public_failure_on_behalf_of_self_non_zero_nonce() {
7070
);
7171
}
7272

73-
#[test(should_fail)] // should_fail_with = "invalid owner"
73+
#[test(should_fail_with = "invalid owner")]
7474
unconstrained fn transfer_in_public_non_existent_nft() {
7575
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough
7676
let (env, nft_contract_address, sender, recipient) =
@@ -83,7 +83,7 @@ unconstrained fn transfer_in_public_non_existent_nft() {
8383
);
8484
}
8585

86-
#[test(should_fail)] // should_fail_with = "unauthorized"
86+
#[test(should_fail_with = "unauthorized")]
8787
unconstrained fn transfer_in_public_failure_on_behalf_of_other_without_approval() {
8888
// Setup with account contracts. Slower since we actually deploy them, but needed for authwits.
8989
let (env, nft_contract_address, sender, recipient, token_id) =
@@ -95,7 +95,7 @@ unconstrained fn transfer_in_public_failure_on_behalf_of_other_without_approval(
9595
);
9696
}
9797

98-
#[test(should_fail)] // should_fail_with = "unauthorized"
98+
#[test(should_fail_with = "unauthorized")]
9999
unconstrained fn transfer_in_public_failure_on_behalf_of_other_wrong_caller() {
100100
// Setup with account contracts. Slower since we actually deploy them, but needed for authwits.
101101
let (env, nft_contract_address, sender, recipient, token_id) =

noir-projects/noir-contracts/contracts/app/nft_contract/src/test/transfer_to_private.nr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ unconstrained fn transfer_to_private_external_orchestration() {
4949
utils::assert_owns_public_nft(env, nft_contract_address, AztecAddress::zero(), token_id);
5050
}
5151

52-
#[test(should_fail)] // should_fail_with = "Invalid partial note or completer"
52+
#[test(should_fail_with = "Invalid partial note or completer")]
5353
unconstrained fn transfer_to_private_transfer_not_prepared() {
5454
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough
5555
let (env, nft_contract_address, owner, _, token_id) =
@@ -66,7 +66,7 @@ unconstrained fn transfer_to_private_transfer_not_prepared() {
6666
);
6767
}
6868

69-
#[test(should_fail)] // should_fail_with = "invalid NFT owner"
69+
#[test(should_fail_with = "invalid NFT owner")]
7070
unconstrained fn transfer_to_private_failure_not_an_owner() {
7171
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough
7272
let (env, nft_contract_address, owner, not_owner, token_id) =
@@ -89,7 +89,7 @@ unconstrained fn transfer_to_private_failure_not_an_owner() {
8989
);
9090
}
9191

92-
#[test(should_fail)] // should_fail_with = "Invalid partial note or completer"
92+
#[test(should_fail_with = "Invalid partial note or completer")]
9393
unconstrained fn incorrect_completer_cannot_complete_partial_note() {
9494
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough.
9595
let (mut env, nft_contract_address, owner, recipient, token_id) =

noir-projects/noir-contracts/contracts/app/token_contract/src/test/access_control.nr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ unconstrained fn access_control() {
2626
assert(!env.view_public(token.is_minter(new_admin)));
2727
}
2828

29-
#[test(should_fail)]
29+
#[test(should_fail_with = "caller is not admin")]
3030
unconstrained fn set_admin_requires_admin_caller() {
3131
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough
3232
let (env, token_contract_address, _, other) = utils::setup(/* with_account_contracts */ false);
@@ -36,7 +36,7 @@ unconstrained fn set_admin_requires_admin_caller() {
3636
let _ = env.call_public(other, token.set_admin(other));
3737
}
3838

39-
#[test(should_fail)]
39+
#[test(should_fail_with = "caller is not admin")]
4040
unconstrained fn set_minter_requires_admin_caller() {
4141
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough
4242
let (env, token_contract_address, _, other) = utils::setup(/* with_account_contracts */ false);

noir-projects/noir-contracts/contracts/app/token_contract/src/test/burn_public.nr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ unconstrained fn burn_public_on_behalf_of_other() {
4343
);
4444
}
4545

46-
#[test(should_fail)] // should_fail_with = "Assertion failed: attempt to subtract with overflow"
46+
#[test(should_fail_with = "Assertion failed: attempt to subtract with overflow")]
4747
unconstrained fn burn_public_failure_more_than_balance() {
4848
let (env, token_contract_address, owner, _, mint_amount) =
4949
utils::setup_and_mint_to_public(/* with_account_contracts */ false);
@@ -57,7 +57,7 @@ unconstrained fn burn_public_failure_more_than_balance() {
5757
);
5858
}
5959

60-
#[test(should_fail)] // should_fail_with = "Assertion failed: Invalid authwit nonce. When 'from' and 'msg_sender' are the same, 'authwit_nonce' must be zero"
60+
#[test(should_fail_with = "Assertion failed: Invalid authwit nonce. When 'from' and 'msg_sender' are the same, 'authwit_nonce' must be zero")]
6161
unconstrained fn burn_public_failure_on_behalf_of_self_non_zero_nonce() {
6262
let (env, token_contract_address, owner, _, mint_amount) =
6363
utils::setup_and_mint_to_public(/* with_account_contracts */ false);
@@ -71,7 +71,7 @@ unconstrained fn burn_public_failure_on_behalf_of_self_non_zero_nonce() {
7171
);
7272
}
7373

74-
#[test(should_fail)] // should_fail_with = "unauthorized"
74+
#[test(should_fail_with = "unauthorized")]
7575
unconstrained fn burn_public_failure_on_behalf_of_other_without_approval() {
7676
let (env, token_contract_address, owner, recipient, mint_amount) =
7777
utils::setup_and_mint_to_public(/* with_account_contracts */ true);
@@ -84,7 +84,7 @@ unconstrained fn burn_public_failure_on_behalf_of_other_without_approval() {
8484
let _ = env.call_public(recipient, burn_call_interface);
8585
}
8686

87-
#[test(should_fail)] // should_fail_with = "unauthorized"
87+
#[test(should_fail_with = "unauthorized")]
8888
unconstrained fn burn_public_failure_on_behalf_of_other_wrong_caller() {
8989
let (env, token_contract_address, owner, recipient, mint_amount) =
9090
utils::setup_and_mint_to_public(/* with_account_contracts */ true);

noir-projects/noir-contracts/contracts/app/token_contract/src/test/mint_to_public.nr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ unconstrained fn mint_to_public_success() {
1717
assert(total_supply == mint_amount);
1818
}
1919

20-
#[test(should_fail)]
20+
#[test(should_fail_with = "caller is not minter")]
2121
unconstrained fn mint_as_non_minter() {
2222
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough
2323
let (env, token_contract_address, _, recipient) =

0 commit comments

Comments
 (0)