This depends on the embedded data PR itself (#28792), until merge this will remain in draft status. All commit but the last one are from there.
There has been interest in exporting the embedded data back into a file. This is implemented here with a simple exportasmap RPC which provides this functionality. The exported file can be used to verify the data integrity or statistical analysis using e.g. contrib/asmap-tool.py.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33920.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
Rebased and taken out of draft status since embedding asmap was merged.
1127 | + "exportasmap", 1128 | + "Export the embedded ASMap data to a file.\n", 1129 | + { 1130 | + {"filepath", RPCArg::Type::STR, RPCArg::Optional::NO, "The path where the ASMap file is saved"}, 1131 | + }, 1132 | + RPCResult{RPCResult::Type::NONE, "", ""},
According to the dev notes:
Try to make the RPC response a JSON object.
Also, could clang-format new code?
Made it return an empty json object and ran clang-format
Tested b065ec9acc98112f4ea9a6d2bae9a7a6187ee2a8
Built with and without embedded asmap. Ran feature_asmap.py, passes in both configurations. Commented out file << asmap_data and confirmed the test fails.
1125 | +{ 1126 | + return RPCHelpMan{ 1127 | + "exportasmap", 1128 | + "Export the embedded ASMap data to a file.\n", 1129 | + { 1130 | + {"filepath", RPCArg::Type::STR, RPCArg::Optional::NO, "The path where the ASMap file is saved"},
should clarify that this will overwrite any prior file?
Added, thanks!
Concept ACK
1134 | + HelpExampleCli("exportasmap", "\"/tmp/asmap.dat\"") + HelpExampleRpc("exportasmap", "\"/tmp/asmap.dat\"")}, 1135 | + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { 1136 | +#ifndef ENABLE_EMBEDDED_ASMAP 1137 | + throw JSONRPCError(RPC_MISC_ERROR, "No embedded ASMap data available"); 1138 | +#else 1139 | + std::span<const std::byte> asmap_data{node::data::ip_asn};
Nit: I guess you are declaring this as an alias to the data? Not sure how useful that is, seems clearer to just point to the embedded data directly.
Sure, done.
1127 | + "exportasmap", 1128 | + "Export the embedded ASMap data to a file.\n", 1129 | + { 1130 | + {"filepath", RPCArg::Type::STR, RPCArg::Optional::NO, "The path where the ASMap file is saved. Existing file will be overwritten."}, 1131 | + }, 1132 | + RPCResult{RPCResult::Type::OBJ, "", ""},
In e.g. dumptxoutset we return a path and some meta information in the result. Might make sense to add that here too?
Ok, returning the path, bytes written and file hash now.
1125 | +{ 1126 | + return RPCHelpMan{ 1127 | + "exportasmap", 1128 | + "Export the embedded ASMap data to a file.\n", 1129 | + { 1130 | + {"filepath", RPCArg::Type::STR, RPCArg::Optional::NO, "The path where the ASMap file is saved. Existing file will be overwritten."},
I think it would be good to clarify if this takes relative and absolute paths, as done in dumptxoutset. Maybe it can just directly mimic the file handling mechanics from that endpoint?
Ok, I aligned file handling with dumptxoutset except for the override part. I think it is fine to override here because this is just single file that comes out of of specific core version and this happens almost instantly. For dumptxoutset the RPC can potentially run a long time and there can be dumps for many different height generated from a single bitcoin core node. Does this distinction make sense to you? I am not feeling super passionate about this though.
Yes, overwriting seems fine to me too.
<!--85328a0da195eb286784d51f73fa0af9-->
🚧 At least one of the CI tasks failed.
<sub>Task FreeBSD Cross: https://github.com/bitcoin/bitcoin/actions/runs/24719866520/job/72305651214</sub>
<sub>LLM reason (✨ experimental): CI failed because the C++ build failed to compile rpc/net.cpp due to an unknown/undeclared RPCHelpMan type.</sub>
<details><summary>Hints</summary>
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
</details>
ACK 2a90b6132a58325e5260dcaaa2560b7f8ad5622e
1165 | + HashWriter hasher; 1166 | + hasher.write(node::data::ip_asn); 1167 | + 1168 | + UniValue result(UniValue::VOBJ); 1169 | + result.pushKV("path", export_path.utf8string()); 1170 | + result.pushKV("bytes_written", (uint64_t)node::data::ip_asn.size());
nit: Should this instead use a functional cast?
Yeah, should have, will try to address this with the next asmap-related code change
Tested ACK 2a90b6132a58325e5260dcaaa2560b7f8ad5622e
Rebuilt with and without embedded asmap and verified feature_asmap.py passes in both builds. One style guide nit, but feel free to ignore.
ACK 2a90b6132a58325e5260dcaaa2560b7f8ad5622e
