chore: document NodeConfig parameters

[Jiloc]

Description

Add documentation for all NodeConfig paramters

Applicable issues

• first step of Document config.toml options automatically #5898

Additional info (benefits, drawbacks, caveats)

Checklist

• Test coverage for new or modified code paths
• Changelog is updated
• Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
• New clarity functions have corresponding PR in clarity-benchmarking repo
• New integration test(s) added to bitcoin-tests.yml

[wileyj]

few minor nits, but overall this is a great start!

[obycode]

Setting this to true is required if you are running a signer, but setting it to true does not require running a signer.

[Jiloc]

changed in e8cca43

[obycode]

I think it would be more correct to say that setting this flag to true sets the node up to replicate the miner and signer Stacker DBs required for signing.

[Jiloc]

changed in e8cca43

[obycode]

Can you also add a note that miner must also be set to true to enable mock-mining.

[Jiloc]

changed in e8cca43

[obycode]

This one is related to mining after new blocks are seen in the network. I'm not quite sure what it should say though.

[Jiloc]

changed in e8cca43

[wileyj]

Suggested change

	/// When `true`, *and* [`NodeConfig::miner`] is also `true`, the node may generate blocks
	/// When `true`, *and* [`node::miner`] is also `true`, the node may generate blocks

i'm not sure if we should document this using the function name vs the toml name of node

[obycode]

or maybe just "and miner is also true", since the context should be clear that it is referring to the above field.

[Jiloc]

That's a really good point about how this will look in the config.toml!

So, the reason I went with [`NodeConfig::miner`] in the Rust comments is because it's super handy for us developers. cargo docs automatically generates links to the fields or even in the IDE and it jumps you straight to where miner is defined. But you're right, we definitely need to think about the final config.toml docs that users will see. Here’s what I’m thinking:

• In the Rust code comments, let's stick to [`StructName::field_name`] everywhere. So, we'd always write [`NodeConfig::miner`] or [`BurnchainConfig::whatever`]. It's just one simple rule to follow.
• Then, we'll need a sort of script for auto-generating the TOML docs from the Configs. The original idea was to have this script ingest directly the rustdoc output. When it's creating the docs for the NodeConfig struct and it sees [`NodeConfig::miner`], it'll just put miner in the TOML doc. Nice and clean for the TOML.

And if we reference something from another struct, say NodeConfig talks about [`BurnchainConfig::some_setting`], the script could turn that into something like [burnchain].some_setting (I am open to better suggestions for it though!) in the TOML. So it clearly points to the right spot in the TOML.

Basically, we keep the full [`StructName::field_name`] in the code for our own benefit (the links!). Then, the script takes care of making the final TOML documentation look good and easy to understand for anyone reading that.
This way, we get those useful developer links and clean TOML docs without having to write comments differently all the time.

What do you all think?

[obycode]

Thanks for explaining. Your plan sounds good to me!

[obycode]

In VS Code, I see that these link to the https://docs.rs site, not to the local definition. If that's the case, we should also make an effort to publish a crate so that our docs are available on https://docs.rs.

[Jiloc]

Glad the plan sounds good! That's interesting about VS Code linking to docs.rs for you. On my setup, when I Ctrl+Click on a link like [`NodeConfig::miner`] directly within the code comments, VS Code with Rust Analyzer jumps directly to the local definition of miner within the NodeConfig struct in my workspace. But maybe I'm misunderstanding the specific context where you saw it?

[obycode]

This looks good, I just had some minor comments.

I think these descriptive comments should also be added to the NodeConfigFile struct, since this would be the one that would be most important to extract into documentation for users. Maybe it is less important to document the default values on NodeConfig and more important to show that on NodeConfigFile, since it would be shown to users trying to understand the config file.

[obycode]

or maybe just "and miner is also true", since the context should be clear that it is referring to the above field.

[obycode]

Microblocks were disabled in 2.5 (See #4561). This applies to other mentions about 3.0 and microblocks.

[Jiloc]

Thanks for the info! I didn't know, I just thought they were deprecated because of Nakamoto! Changed in 723f16e

[obycode]

Let's make this consistent -- either include the (X seconds) on all of the ms parameters, or on none of them.

[Jiloc]

added to all, 723f16e

[obycode]

I think these descriptive comments should also be added to the NodeConfigFile struct, since this would be the one that would be most important to extract into documentation for users. Maybe it is less important to document the default values on NodeConfig and more important to show that on NodeConfigFile, since it would be shown to users trying to understand the config file.

Per discussion in the Nakamoto sync, let's leave the comments as-is for now, and in a separate issue, we will work on merging the two versions of the config structs.

[Jiloc]

I think these descriptive comments should also be added to the NodeConfigFile struct, since this would be the one that would be most important to extract into documentation for users. Maybe it is less important to document the default values on NodeConfig and more important to show that on NodeConfigFile, since it would be shown to users trying to understand the config file.

Per discussion in the Nakamoto sync, let's leave the comments as-is for now, and in a separate issue, we will work on merging the two versions of the config structs.

@obycode Unfortunately, I couldn't attend today's sync :( , but your first comment is interesting because I actually came to a similar conclusion about where the docs might best live. When I worked on ConnectionOptionsFile in PR #6062, I realized that documenting the *File struct directly made more sense for user-facing TOML documentation, especially since the internal ConnectionOptions struct had tens fields not exposed in the config file that didn't make any sense to document this way.

Given that, I'd be open to move the documentation to the *File structs now, I'm happy to make that change for this PR (and the other 2). It should be a quick refactor on my end. This would give us more user-focused docs in the short term.

Of course, I'm also perfectly fine sticking to the plan from the sync if that's preferred, and then we can address it all during the struct merge.

Either way, great idea merging the two struct versions in the long run. It'll definitely simplify things and make the config handling clearer. We'll just have to decine on a strategy for the internal-only fields, but that's for the future discussion.

[Jiloc]

I think these descriptive comments should also be added to the NodeConfigFile struct, since this would be the one that would be most important to extract into documentation for users. Maybe it is less important to document the default values on NodeConfig and more important to show that on NodeConfigFile, since it would be shown to users trying to understand the config file.

Per discussion in the Nakamoto sync, let's leave the comments as-is for now, and in a separate issue, we will work on merging the two versions of the config structs.

@obycode Unfortunately, I couldn't attend today's sync :( , but your first comment is interesting because I actually came to a similar conclusion about where the docs might best live. When I worked on ConnectionOptionsFile in PR #6062, I realized that documenting the *File struct directly made more sense for user-facing TOML documentation, especially since the internal ConnectionOptions struct had tens fields not exposed in the config file that didn't make any sense to document this way.

Given that, I'd be open to move the documentation to the *File structs now, I'm happy to make that change for this PR (and the other 2). It should be a quick refactor on my end. This would give us more user-focused docs in the short term.

Of course, I'm also perfectly fine sticking to the plan from the sync if that's preferred, and then we can address it all during the struct merge.

Either way, great idea merging the two struct versions in the long run. It'll definitely simplify things and make the config handling clearer. We'll just have to decine on a strategy for the internal-only fields, but that's for the future discussion.

As per discussion in today's Naka sync, we'll keep the comments in the "non-file" structs, because the comments will be useful for the developers. We'll try to achieve the merging of the Config structs in the near future.