A copy of the partial signatures is not required before serializing them.
For reference, this was found by switching the codebase to std::span (not sure why it wasn’t found with Span though):
0./psbt.h:240:23: error: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy,-warnings-as-errors]
1 240 | for (auto sig_pair : partial_sigs) {
2 | ^
3 | const &
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Approach ACK. Less copying makes sense. Briefly searched for other instances in psbt.h where potentially unnecessary copying is done.
Would it also make sense to adjust these references to const (or is there a specific reason why const should not be used here)? Adjusted to const auto& entry and ran unit and functional tests (no issues observed).
Less copying makes sense. Briefly searched for other instances in psbt.h where potentially unnecessary copying is done.
The examples you provided are already const references. In C++, when calling a member method that is marked const, member access will be const. Moreover, auto& is a reference that hides the type, including the const-ness, so const auto& and auto& should be identical in this context (const references that do not copy), and no further change should be needed.
no further change should be needed.
Thanks. ACK fab01b5220c28a334b451ed9625bd3914c48e6af