monero icon indicating copy to clipboard operation
monero copied to clipboard

add the right prunable hash per tx to pruned get_blocks.bin call

Open akildemir opened this issue 6 months ago • 3 comments

Fixes https://github.com/monero-project/monero/issues/10120 .

This approach uses the already existing bs return type from find_blockchain_supplement function call instead of fetching the prunable hashes separately. The second of the bs type std::vector<std::pair<crypto::hash, cryptonote::blobdata>> It appears to be used for tx hash and pruned tx blob if pruned is true. However that tx hash is never used, so this approach uses that place for prunable hash and passes it to the return type accordingly.

This behavior might be documented or another approach might have been fetching prunable hashes separately after the find_blockchain_supplement call. This approach is chosen not to make that second call to the db and for its simplicity.

Comments are welcome!

akildemir avatar Oct 02 '25 09:10 akildemir

This adds >32B per transaction for wallet2 which doesn't currently have code implemented to check the TXID using the prunable hash. Before this gets merged, I'd like to see the check on the wallet side implemented, perhaps in a dependent PR. For backwards compatibility, it could skip the check if --allow-mismatched-daemon-version is specified. I could do it but am sort of busy at the moment.

Also, the RPC minor version should to be bumped since a feature was added.

jeffro256 avatar Oct 04 '25 23:10 jeffro256

The current get_blocks.bin pruned response always includes the prunable_hash field, even if not set @jeffro256. Setting it to the correct value, instead of the null hash, would not increase bandwidth and I'd argue it a bug fix, not a feature (though it may still justify a minor version bump).

kayabaNerve avatar Oct 04 '25 23:10 kayabaNerve

Sorry, I was under the impression that null hashes don't get serialized in block_complete_entry, but that's KV_SERIALIZE_VAL_POD_AS_BLOB_OPT, not KV_SERIALIZE_VAL_POD_AS_BLOB. So yeah this takes up the same bandwidth in that case.

jeffro256 avatar Oct 05 '25 06:10 jeffro256