The chain_dir() method of test framework nodes returned the chain name for every non-signet chain, but mainnet stores its data in the top-level data directory. Mainnet tests worked around this by setting self.chain to the empty string, which doubles as the datadir name.
Instead of this workaround, we now set self.chain to "main" and map it to an empty datadir and config section key, so tests can name the chain the way the rest of the codebase does.
This was mentioned in #34566 (review).
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline and AI policy for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | ismaelsadeeq, maflcko, sedited |
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
517 | @@ -518,8 +518,11 @@ def initialize_datadir(dirname, n, chain, disable_autoconnect=True): 518 | 519 | 520 | def write_config(config_path, *, n, chain, extra_config="", disable_autoconnect=True): 521 | - # Translate chain subdirectory name to config name 522 | - if chain == 'testnet3': 523 | + # Translate chain name to config name
In "test: refactor: make chain_dir consistent with CreateBaseChainParams" 0cea924d3326a787ed068d526f7396c6561ff6b9
nit: It is unclear at first why we need to set chain name and chain name conf section to None.
But its because after this change, chain is "main" so the chain_name_conf and chain_name_conf_section needs to be handled else they will have "main" which is not correct. Maybe mention that in the commit message?
Yeah, sorry, the generated commit message was confusing. I let Claude write it, and it phrased it like this:
and translate it to no chain argument and no config section in write_config
I rewrote it in my own words now in a4bc96966f. Hopefully it makes more (human) sense now!
Also updated PR description with the new commit message.
Code review ACK 0cea924d3326a787ed068d526f7396c6561ff6b9
This is a simple PR that makes chain_dir consistent with CreateBaseChainParams.
I have built and tested the PR and verified that there are no other call sites where self.chain="".
The chain_dir() method of test framework nodes returned the chain *name*
for every non-signet chain, but mainnet stores its data in the top-level
data directory. Mainnet tests worked around this by setting self.chain
to the empty string, which doubles as the datadir name.
Instead of this workaround, we now set self.chain to "main" and map it
to an empty datadir and config section key, so tests can name the chain
the way the rest of the codebase does.
reACK a4bc96966faa9efcd7b387907d51b4ac53f7039d
lgtm ACK a4bc96966faa9efcd7b387907d51b4ac53f7039d
ACK a4bc96966faa9efcd7b387907d51b4ac53f7039d