bcoin icon indicating copy to clipboard operation
bcoin copied to clipboard

scripts: fuzz: pass hash as buffer not string

Open pinheadmz opened this issue 6 years ago • 5 comments

One more lingering regression leftover from the great "hashes as buffers instead of strings" refactor from #533

pinheadmz avatar Nov 01 '19 14:11 pinheadmz

Codecov Report

Merging #900 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #900   +/-   ##
=======================================
  Coverage   61.72%   61.72%           
=======================================
  Files         155      155           
  Lines       26058    26058           
=======================================
  Hits        16085    16085           
  Misses       9973     9973

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 77d8804...6346e43. Read the comment docs.

codecov-io avatar Nov 01 '19 14:11 codecov-io

Perhaps we should remove (and not maintain) this file? This script is mostly non-functional in general, as the module nodeconsensus is not available anywhere. It was presumably some form of bindings to Bitcoin Core libbitcoinconsensus library similar to bitcoinconsensus. I looked at this a year or so ago, and used a different approach that was compatible with AFL, I can lookup where the branch is if necessary.

braydonf avatar Nov 01 '19 18:11 braydonf

Hmm, yeah maybe we should remove the /scripts directory:

certs.sh: only relevant for bip70, removed from bcoin a long time ago dump.js: requires npm package heapdump, not a bcoin dependency, dumps a heap snapshot in the repo root directory, not sure what its for. fuzz.js: see above gen.js: needs a new keyword on line 36, otherwise works. Probably useful when the library was first started, but genesis blocks are stored in lib/protocol/networks.js now. seeds.sh: pretty cool script to download seed nodes from bitcoin core repository. Had a bug when run on my shell (head: illegal line count -- -1 but that was fixed by changing line 13 & 14 in the script)

pinheadmz avatar Nov 07 '19 20:11 pinheadmz

certs.sh: True, only relevant for bip70. dump.js: Also not sure how it was used. fuzz.js: Yep, this could become part of fuzz testing setup if maintained. gen.js: True, also tested the fix and it works, but as mentioned may not be necessary. seeds.sh: I didn't have a bug when running. This may be useful, seeds will likely need to be updated.

braydonf avatar Nov 15 '19 00:11 braydonf

so...

certs.sh: remove dump.js: remove fuzz.js: remove? or fix the toString and remove nodeconsensus path? gen.js: remove seeds.sh: keep - may be only truly useful script here

...?

pinheadmz avatar Nov 15 '19 14:11 pinheadmz