Skip to content

refactor: TestWallet API cleanup pt. 1#17283

Merged
benesjan merged 1 commit intonextfrom
09-25-refactor_testwallet_api_cleanup
Sep 26, 2025
Merged

refactor: TestWallet API cleanup pt. 1#17283
benesjan merged 1 commit intonextfrom
09-25-refactor_testwallet_api_cleanup

Conversation

@benesjan
Copy link
Contributor

@benesjan benesjan commented Sep 25, 2025

Followup work of #17228

In this PR I drop getPXEInfo and getContracts from TestWallet + add random docs improvements.

getPXEInfo was just a tech debt as it was only used to check if node has the same protocol contract addresses which is quite non-sensical because those addresses are no literally 1, 2, 3, 4, 5... I assume this check is there from time when they didn't have these hardcoded addresses.

getContracts was used just in one test and the check there was not really that important so I just dropped it.

Copy link
Contributor Author

benesjan commented Sep 25, 2025

- `--sponsored-fpc`: Populate genesis state with a testing sponsored FPC contract
- `--accelerated-test-deployments`: Fire and forget deployment transactions, use in testing only (default: false)

### deploy-l1-verifier
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This command doesn't exist on the cli.

- `--public-deploy`: Publishes the account contract instance (and the class, if needed). Needed if the contract contains public functions.
- `-p, --public-key <string>`: Public key that identifies a private signing key stored outside of the wallet. Used for ECDSA SSH accounts over the secp256r1 curve.
- `-n, --node-url <string>`: URL of the PXE (default: "http://host.docker.internal:8080")
- `-n, --node-url <string>`: URL of Aztec Node (default: "http://host.docker.internal:8080")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Forgot to update in the last PR. These references seems like a pretty bad thing to have given that aztec --help is available and there it will not become stale.

*/
public async register(): Promise<AccountWithSecretKey> {
await this.pxe.registerContract({
await this.wallet.registerContract({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Random change when I was investigating what is remaining from allowing me to drop the ugly getPxe method on test wallet. That function can be dropped once we allow Wallet.registerContract to accept secretKey and partialAddress on the input.

const receipt = await aztecNode.getTxReceipt(txHash);
return receipt.status;
} else {
await inspectTx(wallet, aztecNode, txHash, log, { includeBlockInfo: true });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just did a minor cleanup of this func.

* When this flag is true, simulateTx constructs a request using a fake account (and accepts contract overrides
* on the input) and the PXE emulates kernel effects without generating kernel witnesses. When false, simulateTx
* defers to the standard simulation path.
*/
Copy link
Contributor Author

@benesjan benesjan Sep 25, 2025

Choose a reason for hiding this comment

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

Docs based on comment Grego sent on slack.

Grego pls check it's no ambiguous.

Copy link
Contributor

Choose a reason for hiding this comment

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

The gas estimations not being accurate is not a given, I wouldn't say that in the comment. We aim for them to be accurate and usable, and for now they are

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in d7dd3d0

@benesjan benesjan marked this pull request as ready for review September 25, 2025 11:38
@benesjan benesjan changed the title refactor: TestWallet API cleanup refactor: TestWallet API cleanup pt. 1 Sep 25, 2025
@benesjan benesjan force-pushed the 09-25-refactor_testwallet_api_cleanup branch from 5d56948 to ad7f32e Compare September 25, 2025 12:09
const [secret, messageLeafIndex] = await bridgeL1FeeJuice(
amount,
recipient,
wallet,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wallet no longer needs to be passed in as we can just obtain all the necessary info from node.

executionPayload: ExecutionPayload,
opts: SimulateMethodOptions,
): Promise<TxSimulationResult> {
// In simulated-simulation mode we emulate kernel logic in TS. This is fast but can skew gas,
Copy link
Contributor

Choose a reason for hiding this comment

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

I just dropped this from my current PR, comment is unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nuked it in 3413e33

@benesjan benesjan force-pushed the 09-25-refactor_testwallet_api_cleanup branch from ad7f32e to 3413e33 Compare September 26, 2025 05:10
@benesjan benesjan requested a review from Thunkar September 26, 2025 05:14
Followup work of #17228

In this PR I drop `getPXEInfo` and `getContracts` from `TestWallet` + add random docs improvements.

`getPXEInfo` was just a tech debt as it was only used to check if node has the same protocol contract addresses which is quite non-sensical because those addresses are no literally 1, 2, 3, 4, 5... I assume this check is there from time when they didn't have these hardcoded addresses.

`getContracts` was used just in one test and the check there was not really that important so I just dropped it.
@AztecBot AztecBot force-pushed the 09-25-refactor_testwallet_api_cleanup branch from d7dd3d0 to 94ae864 Compare September 26, 2025 06:12
@benesjan benesjan added this pull request to the merge queue Sep 26, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 26, 2025
@benesjan benesjan added this pull request to the merge queue Sep 26, 2025
Merged via the queue into next with commit df7f19c Sep 26, 2025
16 checks passed
@benesjan benesjan deleted the 09-25-refactor_testwallet_api_cleanup branch September 26, 2025 07:29
AztecBot pushed a commit that referenced this pull request Sep 29, 2025
Continuation of #17283. Changes explained with comments in the code.
github-merge-queue bot pushed a commit that referenced this pull request Sep 29, 2025
Continuation of
#17283. Changes
explained with comments in the code.
mralj pushed a commit that referenced this pull request Oct 13, 2025
Continuation of #17283. Changes explained with comments in the code.
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