test: Add signet witness commitment section parse tests #20004

No description provided.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

No conflicts as of last run.<!--2502f1a698b3751726fa55edcda76cd3-->

Coverage

^{Updated at: 2020-09-29T09:37:30.142032.}

Before: https://drahtbot.github.io/reports/coverage/bitcoin/bitcoin/master/total.coverage/src/signet.cpp.gcov.html After: https://drahtbot.github.io/reports/coverage/bitcoin/bitcoin/20004/total.coverage/src/signet.cpp.gcov.html

cc @kallewoof / @ajtowns :sweat_smile:

Nice!

utACK faee74f6c841eea391c08ebbadb045746c3ef77f

Not really convinced always explicitly passing in an ArgsManager to CreateChainParams is an improvement. Could make it

std::unique_ptr<const CChainParams> CreateChainParams(const std::string& chain, ArgsManager* pargs = nullptr)
{
   ArgsManager& args = (pargs != nullptr ? *pargs : gArgs);
    ...
}

or similar and avoid touching all the other code?

Seems kind of a shame not to add a dummy tx a well to get to 100% line coverage?

avoid touching all the other code

Long term goal is to get rid of the gArgs global in Core (though, not the gui), so I think this patch will be done anyway. Left as is for now.

Seems kind of a shame not to add a dummy tx a well to get to 100% line coverage?

Done :100:

ACK fa29b5ae666bbb4c19188f0dcf8a1ba738aac624

Long term goal is to get rid of the gArgs global in Core (though, not the gui), so I think this patch will be done anyway. Left as is for now.

I think this is overall an improvement, to pass the arguments in. It's also a straightforward, pretty much obviously correct change I'm okay with doing it here.