brunoerg
Added target compactblocks_parse for Bitcoin Core and Rust Bitcoin
Added new target compactblocks_parse for Bitcoin Core and Rust Bitcoin
see commit message for more details.
Seems like a lot of code got touched due to different formatter being used . Would be good to fix that to ease the review process .
Agree, will fix that asap!
Can you please rebase?
Done! Can you approve the workflow? @brunoerg
Just approved it.
I can review this today but needs rebase.
Done. Need workflow approval!
Approved.
Did you get this code from bitcoinfuzz v1 (master branch)?
Yes, the master branch (v1) already had compact blocks parse.
@brunoerg thanks for reviewing, I have made all the changes. Can you approve the workflow!
Not sure why was the CI failing, it was successful on my Ubuntu VM. I have rebased the changes again!
Not sure why was the CI failing, it was successful on my Ubuntu VM. I have rebased the changes again!
The CI seems to be working fine when I change the order of Compact blocks parse in workflow file to above all the tests steps, see below
-
here's the failed job where I haven't changed the order of compact blocks test: https://github.com/i-am-yuvi/bitcoinfuzz/actions/runs/15041239204/job/42273249405
-
here I have changed the order, https://github.com/i-am-yuvi/bitcoinfuzz/actions/runs/15041661058/job/42274595943
Not sure why was the CI failing, it was successful on my Ubuntu VM. I have rebased the changes again!
The CI seems to be working fine when I change the order of Compact blocks parse in workflow file to above all the tests steps, see below
- here's the failed job where I haven't changed the order of compact blocks test: https://github.com/i-am-yuvi/bitcoinfuzz/actions/runs/15041239204/job/42273249405
- here I have changed the order, https://github.com/i-am-yuvi/bitcoinfuzz/actions/runs/15041661058/job/42274595943
Okay found the issue. It was the deserailize_invoice test which cleans the bitcoin-core module, I have updated the compactblocks_parse steps to execute before in e59831eb67ef06f09042bae5efdd709fddb84aa2
Changes since last commit, 3f45d8e4edc116c76583c9890a5e38a47731cbd4:
- Rebased with v2
- In v1 we were returning the size of total transaction but now we are returning the size of serialized after parsing it, same goes with rustbitcoin as suggested by @brunoerg see here.
Another approach came in my mind was hashing the parsed data, not sure about this approach as I feel returning len would be a simpler and better approach for differential fuzzing.