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.

Sign up for GitHub

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

Jump to bottom

feat: add --no-wait flag to certain cli commands #2378

Merged
merged 1 commit into from
Sep 25, 2023
Merged

feat: add --no-wait flag to certain cli commands #2378

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions yarn-project/cli/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,9 @@ export function getProgram(log: LogFn, debugLogger: DebugLogger): Command {
'Optional deployment salt as a hex string for generating the deployment address.',
getSaltFromHexString,
)
// `options.wait` is default true. Passing `--no-wait` will set it to false.
// https://github.com/tj/commander.js#other-option-types-negatable-boolean-and-booleanvalue
.option('--no-wait', 'Skip waiting for the contract to be deployed. Print the hash of deployment transaction')
Copy link
Collaborator

Choose a reason for hiding this comment

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

--no-wait doesn't get used anywhere does it? Don't we just need --wait?

Copy link
Contributor Author

@alexghr alexghr Sep 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The commander package has specific logic to handle boolean flags --foo and --no-foo under the same foo key. I'll add comments pointing to the docs
https://github.com/tj/commander.js#other-option-types-negatable-boolean-and-booleanvalue

LE: @PhilWindle unless we'd want the option to take a boolean parameter e.g. --wait true and --wait false?

Copy link
Collaborator

@PhilWindle PhilWindle Sep 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the link. If I read it correctly we can just add the --no-wait option can't we? Waiting becomes the default and if you type --no-wait then it gets set to false. We shouldn't need to explicitly add the --wait option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good shout! I've refactored the code to remove the --wait flag (implied as true) leaving only --no-wait for the user to pass.

.action(async (abiPath, options: any) => {
const contractAbi = await getContractAbi(abiPath, log);
const constructorAbi = contractAbi.functions.find(({ name }) => name === 'constructor');
Expand All @@ -181,9 +184,14 @@ export function getProgram(log: LogFn, debugLogger: DebugLogger): Command {
const args = encodeArgs(options.args, constructorAbi!.parameters);
debugLogger(`Encoded arguments: ${args.join(', ')}`);
const tx = deployer.deploy(...args).send({ contractAddressSalt: options.salt });
debugLogger(`Deploy tx sent with hash ${await tx.getTxHash()}`);
const deployed = await tx.wait();
log(`\nContract deployed at ${deployed.contractAddress!.toString()}\n`);
const txHash = await tx.getTxHash();
debugLogger(`Deploy tx sent with hash ${txHash}`);
if (options.wait) {
const deployed = await tx.wait();
log(`\nContract deployed at ${deployed.contractAddress!.toString()}\n`);
} else {
log(`\nDeployment transaction hash: ${txHash}\n`);
}
});

program
Expand Down Expand Up @@ -365,7 +373,7 @@ export function getProgram(log: LogFn, debugLogger: DebugLogger): Command {
.requiredOption('-ca, --contract-address <address>', 'Aztec address of the contract.')
.requiredOption('-k, --private-key <string>', "The sender's private key.", PRIVATE_KEY)
.requiredOption('-u, --rpc-url <string>', 'URL of the Aztec RPC', AZTEC_RPC_HOST)

.option('--no-wait', 'Print transaction hash without waiting for it to be mined')
.action(async (functionName, options) => {
const { contractAddress, functionArgs, contractAbi } = await prepTx(
options.contractAbi,
Expand All @@ -381,13 +389,19 @@ export function getProgram(log: LogFn, debugLogger: DebugLogger): Command {
const wallet = await getSchnorrAccount(client, privateKey, privateKey, accountCreationSalt).getWallet();
const contract = await Contract.at(contractAddress, contractAbi, wallet);
const tx = contract.methods[functionName](...functionArgs).send();
await tx.wait();
log('\nTransaction has been mined');
const receipt = await tx.getReceipt();
log(`Transaction hash: ${(await tx.getTxHash()).toString()}`);
log(`Status: ${receipt.status}\n`);
log(`Block number: ${receipt.blockNumber}`);
log(`Block hash: ${receipt.blockHash?.toString('hex')}`);
if (options.wait) {
await tx.wait();

log('Transaction has been mined');

const receipt = await tx.getReceipt();
log(`Status: ${receipt.status}\n`);
log(`Block number: ${receipt.blockNumber}`);
log(`Block hash: ${receipt.blockHash?.toString('hex')}`);
} else {
log('\nTransaction pending. Check status with get-tx-receipt');
Copy link
Collaborator

Choose a reason for hiding this comment

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

You will need to provide the transaction hash in order for them to use the receipt.

}
});

program
Expand Down