Skip to content

refactor: TestWallet API cleanup pt.2#17295

Merged
benesjan merged 1 commit intonextfrom
09-25-refactor_testwallet_api_cleanup_pt.2
Sep 29, 2025
Merged

refactor: TestWallet API cleanup pt.2#17295
benesjan merged 1 commit intonextfrom
09-25-refactor_testwallet_api_cleanup_pt.2

Conversation

@benesjan
Copy link
Contributor

@benesjan benesjan commented Sep 25, 2025

Continuation of #17283. Changes explained with comments in the code.

Copy link
Contributor Author

benesjan commented Sep 25, 2025

@benesjan benesjan changed the base branch from 09-25-refactor_testwallet_api_cleanup to graphite-base/17295 September 25, 2025 13:37
@benesjan benesjan force-pushed the 09-25-refactor_testwallet_api_cleanup_pt.2 branch from e3b44a5 to 193cfac Compare September 25, 2025 13:37
@benesjan benesjan changed the base branch from graphite-base/17295 to 09-25-refactor_merging_pxe_interface_and_its_implementation September 25, 2025 13:37
@benesjan benesjan force-pushed the 09-25-refactor_testwallet_api_cleanup_pt.2 branch 3 times, most recently from 4413d51 to 206c56d Compare September 26, 2025 05:07
@benesjan benesjan force-pushed the 09-25-refactor_merging_pxe_interface_and_its_implementation branch from a7b9b55 to 62ef624 Compare September 26, 2025 05:10
@benesjan benesjan force-pushed the 09-25-refactor_testwallet_api_cleanup_pt.2 branch from 206c56d to 48e0395 Compare September 26, 2025 05:10
@benesjan benesjan force-pushed the 09-25-refactor_merging_pxe_interface_and_its_implementation branch from 62ef624 to 3c3c721 Compare September 26, 2025 05:14
@benesjan benesjan force-pushed the 09-25-refactor_testwallet_api_cleanup_pt.2 branch 4 times, most recently from e5a2094 to 78097e5 Compare September 26, 2025 07:33
@benesjan benesjan force-pushed the 09-25-refactor_merging_pxe_interface_and_its_implementation branch from e839241 to 3c4c58e Compare September 26, 2025 07:33
@benesjan benesjan force-pushed the 09-25-refactor_testwallet_api_cleanup_pt.2 branch from 78097e5 to de8c543 Compare September 26, 2025 08:24
Comment on lines +112 to +114
get address() {
return this.instance.address;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in DMs, this getter just looks nicer in code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll probably kill the one above soon, since we're "starting from scratch" on wallet apis

--bot.senderSalt <value> ($BOT_ACCOUNT_SALT)
The salt to use to deploys the sender account.

--bot.recipientEncryptionSecret <value> (default: [Redacted]) ($BOT_RECIPIENT_ENCRYPTION_SECRET)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alex confirmed that dropping this and just using arbitrary recipient is fine.

@AztecBot AztecBot force-pushed the 09-25-refactor_merging_pxe_interface_and_its_implementation branch from 1737485 to 89729a0 Compare September 26, 2025 09:04
Base automatically changed from 09-25-refactor_merging_pxe_interface_and_its_implementation to next September 26, 2025 09:52
@benesjan benesjan force-pushed the 09-25-refactor_testwallet_api_cleanup_pt.2 branch from 5ec271b to e50e100 Compare September 26, 2025 10:15
@@ -189,7 +189,7 @@ No correctness is guaranteed on the result of `simulate`! Correct execution is e

This creates and returns a transaction request, which includes proof of correct private execution and side-effects. The request is not broadcast however, and no gas is spent. It is typically used in testing contexts to inspect transaction parameters or to check for execution failure.

#include_code local-tx-fails /yarn-project/end-to-end/src/guides/dapp_testing.test.ts typescript
#include_code local-tx-fails /yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts typescript
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped the dapp_testing.test.ts because it was stale and basically no longer used in docs. This was the only occurrence in docs.

What motivated me to drop it is that we directly obtained there the notes from wallet and that is deprecated.

@benesjan benesjan marked this pull request as ready for review September 26, 2025 10:18
Comment on lines +98 to +101
await wallet.registerKeysForEscrowContract(
crowdfundingSecretKey,
await computePartialAddress(crowdfundingInstance),
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed the problematic function to make it less cancerous before it gets completely nuked.


it('reuses the same account and token contract', async () => {
const { defaultAccountAddress, token, recipient } = bot;
it('reuses the same token contract', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recipient is no longer stable across bot instantiations but Alex said it doesn't matter.

@benesjan benesjan requested a review from Thunkar September 26, 2025 12:10
* Returns the PXE.
* @deprecated This is only used by account manager to because there we call registerAccount. This can be dropped
* once we allow Wallet.registerContract accepts secretKey and partialAddress on the input.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm killing this next, so we should be good

@benesjan benesjan force-pushed the 09-25-refactor_testwallet_api_cleanup_pt.2 branch from 86f95ca to 273de06 Compare September 29, 2025 11:00
@benesjan benesjan enabled auto-merge September 29, 2025 11:42
Continuation of #17283. Changes explained with comments in the code.
@AztecBot AztecBot force-pushed the 09-25-refactor_testwallet_api_cleanup_pt.2 branch from 273de06 to 5b593d6 Compare September 29, 2025 11:45
@benesjan benesjan added this pull request to the merge queue Sep 29, 2025
auto-merge was automatically disabled September 29, 2025 12:32

Pull Request is not mergeable

Merged via the queue into next with commit d1b5b34 Sep 29, 2025
16 checks passed
@benesjan benesjan deleted the 09-25-refactor_testwallet_api_cleanup_pt.2 branch September 29, 2025 12:33
ludamad pushed a commit that referenced this pull request Dec 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants