Page MenuHomePhabricator

[avalanche] Process AvalancheResponse
ClosedPublic

Authored by deadalnix on Apr 10 2020, 23:04.

Details

Summary

Dry run only for now.

Test Plan

Updated unit tests to do several poll/response cycles.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
avaresponse
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 10173
Build 18172: Default Diff Build & Tests
Build 18171: arc lint + arc unit

Event Timeline

jasonbcox requested changes to this revision.Apr 11 2020, 00:04
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/net_processing.cpp
3431

Consider using an intermediate variable for this value and using it in both places.

3454

I guess not relevant to this diff, but this misbehavior case clearly isn't tested.

3505

s/verctor/vector

3518

You added two new ban cases. Test for them.

This revision now requires changes to proceed.Apr 11 2020, 00:04
deadalnix added inline comments.
src/net_processing.cpp
3431

Which are mutually exclusive.

3518

I don't really see the value in testing behavior that is quite clearly incorrect.

src/net_processing.cpp
3431

I'm referring only to the gArgs.GetBoolArg("-enableavalanche", AVALANCHE_DEFAULT_ENABLED) part.

3518

I can see the argument for invalid-ava-response-content but not for invalid-ava-response-signature

src/net_processing.cpp
3431

Do you ever expect strCommand to be both equal to NetMsgType::AVAPOLL and NetMsgType::AVARESPONSE at the same time?

jasonbcox added inline comments.
src/net_processing.cpp
3431

Reread my latest comment ^^:

I'm referring only to the gArgs.GetBoolArg("-enableavalanche", AVALANCHE_DEFAULT_ENABLED) part.

But I'm not going to block this patch on this any longer.

This revision is now accepted and ready to land.Apr 15 2020, 00:56
This revision was automatically updated to reflect the committed changes.