bosagora / agora

POC Node implementation for CoinNet
https://bosagora.io
MIT License
37 stars 22 forks source link

Add new validator to whitelist #3218

Closed linked0 closed 2 years ago

linked0 commented 2 years ago

In the case of the genesis validators, there is no address registered right after nodes start from scratch, which means the genesis validators might be banned although there have been the whitelist calls for the validators in the middle of accepting the Genesis block. So we should add the UTXOs to the whitelist when the handshaking has been completed.

Fixes #3177

codecov[bot] commented 2 years ago

Codecov Report

Merging #3218 (c39ce9f) into v0.x.x (3169ed2) will increase coverage by 51.50%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           v0.x.x    #3218       +/-   ##
===========================================
+ Coverage   27.84%   79.35%   +51.50%     
===========================================
  Files         304      212       -92     
  Lines       29023    19119     -9904     
===========================================
+ Hits         8082    15171     +7089     
+ Misses      20941     3948    -16993     
Flag Coverage Δ
integration 20.14% <ø> (-7.71%) :arrow_down:
unittests 87.48% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
source/agora/network/Manager.d 79.80% <100.00%> (+13.68%) :arrow_up:
source/agora/common/VibeTask.d 0.00% <0.00%> (-86.67%) :arrow_down:
source/agora/common/FileBasedLock.d 0.00% <0.00%> (-72.73%) :arrow_down:
source/agora/network/RPC.d 1.51% <0.00%> (-59.85%) :arrow_down:
source/agora/network/VibeManager.d 1.42% <0.00%> (-44.29%) :arrow_down:
source/agora/node/Runner.d 0.00% <0.00%> (-43.07%) :arrow_down:
source/scpd/scp/SCPDriver.d 60.00% <0.00%> (-15.00%) :arrow_down:
source/agora/consensus/Quorum.d 97.09% <0.00%> (-0.28%) :arrow_down:
source/scpd/scp/SCP.d 100.00% <0.00%> (ø)
source/agora/crypto/Crc16.d 100.00% <0.00%> (ø)
... and 305 more

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 3169ed2...c39ce9f. Read the comment docs.

linked0 commented 2 years ago

MacOS is green.

Geod24 commented 2 years ago

Doesn't this whitelist just about every connection we manage to establish though ? If I read the bug's log correctly, it looks like we whitelist the hostname but ban the IP ?

linked0 commented 2 years ago

Doesn't this whitelist just about every connection we manage to establish though ?

It whitelists every address which has been connected, which is the goal of this call (as follows) but failed because there was no connection yet.

validators.each!(validator => this.network.whitelist(validator.utxo));

If I read the bug's log correctly, it looks like we whitelist the hostname but ban the IP ?

No, we ban the hostname as follows.

2022-03-10 07:09:30,332 Info [agora.common.BanManager] - BanManager: Address agora://eu-002.bosagora.io/ banned at unixtime:1646896170 (2022-Mar-10 07:09:30Z) until unixtime:1646982570 (2022-Mar-11 07:09:30Z)
2022-03-10 07:09:30,929 Info [agora.common.BanManager] - BanManager: Address agora://eu-002.bosagora.io:3826/ banned at unixtime:1646896170 (2022-Mar-10 07:09:30Z) until unixtime:1646982570 (2022-Mar-11 07:09:30Z)
...

I verified that this change makes a node start catch-up in the testnet. Otherwise, the genesis validators are banned in the end and the messages like Could not perform catchup yet because we have no peer are repeated.