Page MenuHomePhabricator

[avalanche] Delegate node management to caller
ClosedPublic

Authored by deadalnix on Mon, Nov 26, 23:39.

Details

Summary

So far, AvalancheProcessor tried to find apropriate nodes by itself. This diff delegates the responsability to the caller, which will add peer usables for avalanche using the addPeer method.

Now peers are stored in a datastructure tracking the data relevent for the peer and stored in a multi_index in order to be able to query quickly the nodes that are available for polling.

Depends on D2137

Test Plan

Updated tests to reflect behavioral changes.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

deadalnix created this revision.Mon, Nov 26, 23:39
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Nov 26, 23:39
Herald added a subscriber: schancel. · View Herald Transcript
jasonbcox edited the summary of this revision. (Show Details)Tue, Nov 27, 00:34
jasonbcox requested changes to this revision.Tue, Nov 27, 00:51
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/avalanche.cpp
80 ↗(On Diff #6121)

tha -> that

84 ↗(On Diff #6121)

This needs to enforce a minimum cooldown of at *least* AVALANCHE_TIME_STEP_MILLISECONDS + 1 (or some other, safer value) so that runEventLoop doesn't get stuck looping forever.

276 ↗(On Diff #6121)

Probably best to introduce a constant called INVALID_NODE or NODE_UNKNOWN for -1.

295 ↗(On Diff #6121)

use the constant here

This revision now requires changes to proceed.Tue, Nov 27, 00:51
deadalnix marked an inline comment as done.Tue, Nov 27, 13:37
deadalnix added inline comments.
src/avalanche.cpp
84 ↗(On Diff #6121)

Why would it get stuck ?

deadalnix updated this revision to Diff 6129.Tue, Nov 27, 13:40

Fix typo

Fabien edited the summary of this revision. (Show Details)Tue, Nov 27, 14:03
Fabien added a subscriber: Fabien.Tue, Nov 27, 17:23
Fabien added inline comments.
src/avalanche.cpp
79 ↗(On Diff #6129)

recieved => received

jasonbcox added inline comments.Tue, Nov 27, 17:58
src/avalanche.cpp
84 ↗(On Diff #6121)

I misread. I was thinking that a small enough cooldown in an AvalancheResponse could get the loop in runEventLoop() to execute on the same nodeId multiple times. Looks like the test at // Running two iterration of the event loop so that vote gets triggerd on A // and B. covers this case though, so we're good.

jasonbcox requested changes to this revision.Tue, Nov 27, 22:23

Back on your queue because tests fail.

src/avalanche.cpp
276 ↗(On Diff #6121)

I still think this should be a constant.

295 ↗(On Diff #6121)

ping

This revision now requires changes to proceed.Tue, Nov 27, 22:23
deadalnix updated this revision to Diff 6161.Thu, Nov 29, 15:02

Add NO_NODE constant instead of using magic numbers.
Fix typos

Fabien added inline comments.Thu, Nov 29, 15:52
src/avalanche.h
39 ↗(On Diff #6161)

What do you think of moving this to net.h ? This definition could be useful outside of avalanche.

deadalnix marked an inline comment as done.Thu, Nov 29, 16:04
deadalnix added inline comments.
src/avalanche.h
39 ↗(On Diff #6161)

That convention is not used through the codebase. If it become the case, then yes. Until then, defining things with a limited scope is better.

Fabien accepted this revision as: Fabien.Thu, Nov 29, 16:05
jasonbcox accepted this revision.Thu, Nov 29, 17:08
This revision is now accepted and ready to land.Thu, Nov 29, 17:08
This revision was automatically updated to reflect the committed changes.