Skip to content

fix: boolean config helper for cli args works now#13110

Merged
just-mitch merged 1 commit intomasterfrom
12988-test-accounts-hard-to-disable
Mar 28, 2025
Merged

fix: boolean config helper for cli args works now#13110
just-mitch merged 1 commit intomasterfrom
12988-test-accounts-hard-to-disable

Conversation

@just-mitch
Copy link
Collaborator

@just-mitch just-mitch commented Mar 27, 2025

Using booleanConfigHelper without [value] in the commander flag was resulting in all our boolean cli args either using the default (if no value was provided), or to false, because the "value" supplied to the parse function was always undefined.

This resuled in, e.g. passing --p2p-enabled being interpreted as false.

Now the expected behavior reigns: --p2p-enabled and --p2p-enabled=true and --p2p-enabled true all result in the flag being true, but --p2p-enabled=false and --p2p-enabled false result in the flag being false.

In the testAccounts case, since it was defined as true by default for the sandbox, it was picking up that value when parsing for the node, so no matter what it was true.

@just-mitch just-mitch linked an issue Mar 27, 2025 that may be closed by this pull request
bug: --test-accounts is hard to turn off #12988
Closed
@just-mitch
fix: boolean config helper for cli args works now
45067e1 
chore: test accounts are on by default in sandbox, off by default otherwise
@just-mitch just-mitch force-pushed the 12988-test-accounts-hard-to-disable branch from 3c61056 to 45067e1 Compare March 27, 2025 23:56
@Maddiaa0
Copy link
Member

Maddiaa0 commented Mar 28, 2025 •
edited
Loading

We may benefit from cli parsing unit tests

PhilWindle
PhilWindle previously requested changes Mar 28, 2025
View reviewed changes
Copy link
Collaborator

@PhilWindle PhilWindle left a comment

Choose a reason for hiding this comment

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

Can you do this into alpha-testnet instead? The test accounts vars have been changed in that branch.

@just-mitch just-mitch dismissed PhilWindle’s stale review March 28, 2025 12:53

I am going to immediately backport this fix to the testnet-alpha branch as well.

@just-mitch just-mitch merged commit a93ce6e into master Mar 28, 2025
7 checks passed
@just-mitch just-mitch deleted the 12988-test-accounts-hard-to-disable branch March 28, 2025 12:53
@AztecBot AztecBot mentioned this pull request Mar 28, 2025
Merged
just-mitch added a commit that referenced this pull request Mar 28, 2025
@just-mitch
fix: boolean config helper for cli args works now (#13110)
a77fbc7 
Using `booleanConfigHelper` without `[value]` in the commander flag was
resulting in all our boolean cli args either using the default (if no
value was provided), or to `false`, because the "value" supplied to the
`parse` function was always `undefined`.

This resuled in, e.g. passing `--p2p-enabled` being interpreted as
false.

Now the expected behavior reigns: `--p2p-enabled` and
`--p2p-enabled=true` and `--p2p-enabled true` all result in the flag
being `true`, but `--p2p-enabled=false` and `--p2p-enabled false` result
in the flag being `false`.

In the `testAccounts` case, since it was defined as `true` by default
for the sandbox, it was picking up that value when parsing for the node,
so no matter what it was true.
charlielye pushed a commit that referenced this pull request Apr 3, 2025
@AztecBot
chore(master): release 0.83.0 (#13112)
c99fb41 
🤖 I have created a new Aztec Packages release
---


##
[0.83.0](v0.82.3...v0.83.0)
(2025-04-03)


### ⚠ BREAKING CHANGES

* operation mouthwash
([#13171](#13171))
* change version and protocol version to rollupVersion
([#13145](#13145))
* processing events in Aztec.nr
([#12957](#12957))

### Features

* accept multiple consensus client endpoints
([#13022](#13022))
([1d0185d](1d0185d))
* add ecdsa non ssh account to cli wallet
([#13085](#13085))
([c5f9984](c5f9984))
* Add parent log link to top of CI logs.
([#13170](#13170))
([a851087](a851087))
* Add public data read gadget
([#13138](#13138))
([6bb76db](6bb76db))
* **avm:** merkle db hints (part 3)
([#13199](#13199))
([7f96676](7f96676))
* **Barretenberg:** static analyzer's routine
([#13207](#13207))
([84890c2](84890c2))
* derived pending notes capsules slot
([#13102](#13102))
([6307ba0](6307ba0))
* get mana limit from rollup by default.
([#13029](#13029))
([a406c54](a406c54))
* Increase CIVC depth with no rollup cost
([#13106](#13106))
([3a555ef](3a555ef))
* making SyncDataProvider throw before sync
([#13151](#13151))
([9840241](9840241))
* more benchmarks
([#13103](#13103))
([7a2c9b7](7a2c9b7))
* **noir:** Allow missing optional fields in msgpack
([#13141](#13141))
([493dede](493dede))
* processing events in Aztec.nr
([#12957](#12957))
([88c0e04](88c0e04))
* Prover id defaults to publisher address
([#13206](#13206))
([f459c3e](f459c3e))
* purge of log decryption in TS
([#12992](#12992))
([800ab8d](800ab8d))
* register private-only contracts in cli-wallet and misc improvements
([#13245](#13245))
([ad0530f](ad0530f))
* remove unary trick in decomposition and constraints polishing
([#13080](#13080))
([0e60255](0e60255))
* split public inputs from proof
([#12816](#12816))
([ca7151e](ca7151e))
* use magic address for fee juice portal in msgs
([#13241](#13241))
([6fbc378](6fbc378))
* util for computing proposer/forwarder address
([#13169](#13169))
([87809b2](87809b2))
* Zw/goblin avm tests
([#12904](#12904))
([baea4b3](baea4b3))


### Bug Fixes

* add check for rollup version in tx validator
([#13197](#13197))
([c8220c9](c8220c9)),
closes
[#13192](#13192)
* add flake
([13bdfa7](13bdfa7))
* add flakes
([547d50a](547d50a))
* **avm:** alu interface
([#13115](#13115))
([101ff78](101ff78))
* await transaction to make proposal
([#13132](#13132))
([180db9e](180db9e))
* bb mac publish on tag push
([#13174](#13174))
([8d46c8c](8d46c8c))
* bb workdir permission issue
([#13095](#13095))
([3685a80](3685a80))
* **bb:** dont publish .zst
([#13173](#13173))
([4dba4e9](4dba4e9))
* boolean config helper for cli args works now
([#13110](#13110))
([a93ce6e](a93ce6e))
* **docs:** Register FPC docs, contract deployment, events
([#13222](#13222))
([49aabfb](49aabfb))
* don't log sepolia acc mnemonic on github action
([#13178](#13178))
([809b44d](809b44d))
* eq instead of !==
([#13161](#13161))
([921e347](921e347))
* fetch the correct vk in getSolidityVerifier
([#13157](#13157))
([71b6719](71b6719))
* force anvil/blob networking to ipv4 on localhost. attempt to fix port
flakes
([#13099](#13099))
([970dae5](970dae5))
* fuzzing build issues
([#13114](#13114))
([bacae3d](bacae3d))
* Handle proven chain events referring to unseen blocks
([#13144](#13144))
([229515f](229515f)),
closes
[#13142](#13142)
* handling multiple identical logs in a tx
([#13184](#13184))
([c100499](c100499))
* incorrect blocknumber in syncTaggedLogs
([e58ea95](e58ea95))
* indexeddb multimap dupes
([#13254](#13254))
([1e470f0](1e470f0))
* load two more points
([#13119](#13119))
([2a2904a](2a2904a))
* Masternet to run with the nightly tag
([#13239](#13239))
([f16b70e](f16b70e))
* newline weirdness in package.json
([#13111](#13111))
([244ea99](244ea99))
* nightly deploy quotes
([#13253](#13253))
([775eecf](775eecf))
* prover config read from L1
([#13237](#13237))
([13426fe](13426fe))
* Race condition while unwinding blocks
([#13148](#13148))
([1c2291a](1c2291a))
* read and pass rollup version
([#13232](#13232))
([4ee1e4a](4ee1e4a))
* recursive sumcheck bugs
([#12885](#12885))
([a784802](a784802))
* reenable test cache
([4dfca94](4dfca94))
* remove [skip ci] edge condition
([#13228](#13228))
([4e699ea](4e699ea))
* separator in pending partial notes capsule array slot
([#13153](#13153))
([0d6ec63](0d6ec63))
* sysdig oops
([5cf6ad1](5cf6ad1))
* tagging bug
([#13061](#13061))
([280e1a6](280e1a6))
* test tracking failures in merge queue
([#13235](#13235))
([650fdc1](650fdc1))
* title and external check need to run in merge queue
([e326ed6](e326ed6))
* Transpile cmov
([#13194](#13194))
([6b94555](6b94555))
* trying to fix EADDRINUSE
([#13176](#13176))
([260a057](260a057))
* Uninstall gossipsub event handler on service stop
([#13190](#13190))
([081f30d](081f30d))
* use version from registry for rollup instead of config
([#12938](#12938))
([7dac390](7dac390))
* validate private double spends in txs with public funcs
([#13088](#13088))
([4555871](4555871))
* yolo flake and alert fix
([f8de52b](f8de52b))


### Miscellaneous

* add container id to netlog
([9ecdf15](9ecdf15))
* Add e2e test to bootstrap test_cmds
([#13146](#13146))
([09e4722](09e4722))
* add rollup version as universal cli option
([#13205](#13205))
([#13213](#13213))
([568d9e9](568d9e9))
* Add ultra versions of fuzzers in stdlib
([#13139](#13139))
([aea210b](aea210b))
* align TS and C++ indexed leaf types
([#13185](#13185))
([d2574cc](d2574cc))
* Assign bb test flake
([#13127](#13127))
([69fdb04](69fdb04))
* **avm:** remove check_interaction from tests
([#13136](#13136))
([7d875a6](7d875a6))
* change version and protocol version to rollupVersion
([#13145](#13145))
([24d7f8b](24d7f8b))
* Cleanup config vars for alpha-testnet
([#13204](#13204))
([153a606](153a606))
* convenient way to run app ivc from bb
([#13158](#13158))
([cb1a857](cb1a857))
* Cron snapshot upload in spartan
([#13108](#13108))
([7c520a8](7c520a8))
* Do not close store before stopping p2p client in tests
([#13223](#13223))
([63a843e](63a843e))
* docker flake diagnostics.
([48da272](48da272))
* **docs:** alpha-testnet versioning
([#13016](#13016))
([9ca5d3c](9ca5d3c))
* **docs:** Update CLI faucet command
([#13104](#13104))
([6eb71de](6eb71de))
* **docs:** Update versions-updating.md
([#13090](#13090))
([0310d4e](0310d4e))
* Enable debug logging for annoying unit tests
([#13191](#13191))
([5556524](5556524))
* enable sentinel in sentinel test
([#13219](#13219))
([f2cfe3f](f2cfe3f))
* Exclude nightly versions from stable fn
([#13196](#13196))
([a832e90](a832e90))
* Fix flake in e2e fees failures
([#13229](#13229))
([5defe47](5defe47))
* fix kind logs
([#13023](#13023))
([c6c2727](c6c2727)),
closes
[#13053](#13053)
* fuzzing build in ci
([#13105](#13105))
([1c08d38](1c08d38))
* Improve callstacks for public dispatch fns
([#13120](#13120))
([f67375d](f67375d))
* log out the slash factory when a new rollup is deployed
([#13131](#13131))
([4643a31](4643a31))
* Merge alpha back to master
([#13128](#13128))
([504c338](504c338))
* metric attributes
([#13126](#13126))
([f87d5e3](f87d5e3)),
closes
[#13063](#13063)
* minor tagging API improvement
([#13092](#13092))
([f5bcece](f5bcece))
* more benchmarking
([#13211](#13211))
([4b0a4ad](4b0a4ad))
* move unbound impl generics to their functions
([#13147](#13147))
([62f497f](62f497f))
* operation mouthwash
([#13171](#13171))
([384f4e5](384f4e5))
* remove catch-all branch from opcode match in transpiler
([#13210](#13210))
([2bc9ca2](2bc9ca2))
* remove templating by flavor in merge protocol
([#13098](#13098))
([cf5e217](cf5e217))
* rename journal dir and file to state manager & mv to up to public/
([#13159](#13159))
([f13be09](f13be09))
* replace relative paths to noir-protocol-circuits
([f18336d](f18336d))
* replace relative paths to noir-protocol-circuits
([d620c12](d620c12))
* replace relative paths to noir-protocol-circuits
([be3e74e](be3e74e))
* replace relative paths to noir-protocol-circuits
([5bfac63](5bfac63))
* replace relative paths to noir-protocol-circuits
([f41ab2c](f41ab2c))
* replace relative paths to noir-protocol-circuits
([806e560](806e560))
* streamlined log processing
([#13107](#13107))
([b184865](b184865))
* use testnet optimized trace
([#13135](#13135))
([5a4f2ac](5a4f2ac))
* uses a new larger redis cache. flake file tweaks.
([#13167](#13167))
([0951fb6](0951fb6))


### Documentation

* Add intermediate migration note versions
([#13183](#13183))
([f5de7b8](f5de7b8))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Maddiaa0 Maddiaa0 Maddiaa0 approved these changes

@PhilWindle PhilWindle PhilWindle left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: --test-accounts is hard to turn off

3 participants

@just-mitch @Maddiaa0 @PhilWindle