Page MenuHomePhabricator

[avalanche] Ensure nullptr safety on proccesor's public interface
ClosedPublic

Authored by sdulfari on May 14 2022, 13:15.

Details

Summary

This needs fixing because I managed to trigger a segfault while writing a separate patch.

Test Plan
ninja check-avalanche

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien added inline comments.
src/avalanche/processor.cpp
323 ↗(On Diff #33528)

Please explain why this is needed: the CBlockIndexWorkComparator is dereferencing the pointer, which causes the find to segfault

src/avalanche/processor.cpp
323 ↗(On Diff #33528)

We have two choices:

  1. Modify CBlockIndexWorkComparator to treat nullptr as least work
  2. Not call the comparator in the first place (this patch)

I chose to not go with option 1 because I did not want to touch validation code. However making that change would mean less worrying about it in multiple call sites here, so it may be worth revisiting.

src/avalanche/processor.cpp
323 ↗(On Diff #33528)

I'm OK with the patch, I just want you to add a comment to explain that the nullptr dereference issue is from the comparator and not the map, so that someone reading this code won't remove the check by mistake, as it's not obvious from the code.

src/avalanche/processor.cpp
291 ↗(On Diff #33588)

Here it's IsWorthPolling that needs the proof to be non null

This revision is now accepted and ready to land.May 18 2022, 16:47