Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix DMN operator connectivity check and local address lookup #2742

Merged
merged 5 commits into from
Feb 20, 2022
Merged

Fix DMN operator connectivity check and local address lookup #2742

merged 5 commits into from
Feb 20, 2022

Conversation

furszy
Copy link

@furszy furszy commented Feb 10, 2022 •
edited
Loading

Correcting two issues on the Deterministic Masternode operator side:

  1. The DMN service address connectivity check was not being performed (due a non-initialized socket), failing all-the-time on testnet and mainnet. Erring out at startup.

  2. CActiveMasternodeManager::GetLocalAddress will prefer IPv4 if there are multiple addresses available locally. --> Fix CActiveMasternodeManager::GetLocalAddress to prefer IPv4 if multiple local addresses are known dashpay/dash#3304.

  3. CActiveMasternodeManager::Init connectivity check was missing to set the error cause.

Plus, now that i'm here, ported bitcoin#12569 as well.

UdjinM6 and others added 4 commits February 10, 2022 12:28
@UdjinM6 @furszy
Fix CActiveMasternodeManager::GetLocalAddress to prefer IPv4 if multi…
34363a5 
…ple local addresses are known
@furszy
Fix CActiveMasternodeManager connectivity test
889ce4a 
Need to create the socket before try to connect it.
@practicalswift @furszy
increase signal-to-noise ratio in debug.log by adjusting log level wh…
6f4e17c 
…en logging failed non-manual connect():s

Before this patch:

```
$ src/bitcoind -printtoconsole
…
2018-02-28 18:42:51 UpdateTip: new best=0000000000005448b10a219683d34b770a28044e1cc421032dea1a79ff548948 height=1286903 version=0x20000000 log2_work=69.791313 tx=17408546 date='2018-02-28 18:42:46' progress=1.000000 cache=0.0MiB(173txo)
2018-02-28T18:37:52Z connect() 10.11.21.34:18333 failed after select(): Connection refused (111)
2018-02-28 18:43:22 connect() to 10.11.43.14:18333 failed after select(): Network is unreachable (101)
2018-02-28 18:44:49 UpdateTip: new best=000000000000029a521ff2803e1441b09413b876accff5084a4cccf7747d798b height=1286904 version=0x20000000 log2_work=69.791345 tx=17408559 date='2018-02-28 18:44:51' progress=1.000000 cache=0.1MiB(502txo)
2018-02-28 18:46:54 connect() to [2001:0:9d38:78ff:1234:1234:1234:1234]:18333 failed: Network is unreachable (101)
2018-02-28 18:48:56 connect() to [2001:0:9d38:6aff:1234:1234:1234:1234]:18333 failed: Network is unreachable (101)
2018-02-28 18:49:11 UpdateTip: new best=000000000000000206b79eb235e5dd907b6369de0e5d764330bf40ec0d460311 height=1286905 version=0x20000000 log2_work=69.791377 tx=17408577 date='2018-02-28 18:49:12' progress=1.000000 cache=1.0MiB(5245txo)
```

After this patch:

```
$ src/bitcoind -printtoconsole
…
2018-02-28 18:42:51 UpdateTip: new best=0000000000005448b10a219683d34b770a28044e1cc421032dea1a79ff548948 height=1286903 version=0x20000000 log2_work=69.791313 tx=17408546 date='2018-02-28 18:42:46' progress=1.000000 cache=0.0MiB(173txo)
2018-02-28 18:44:49 UpdateTip: new best=000000000000029a521ff2803e1441b09413b876accff5084a4cccf7747d798b height=1286904 version=0x20000000 log2_work=69.791345 tx=17408559 date='2018-02-28 18:44:51' progress=1.000000 cache=0.1MiB(502txo)
2018-02-28 18:49:11 UpdateTip: new best=000000000000000206b79eb235e5dd907b6369de0e5d764330bf40ec0d460311 height=1286905 version=0x20000000 log2_work=69.791377 tx=17408577 date='2018-02-28 18:49:12' progress=1.000000 cache=1.0MiB(5245txo)
```

Please note that "manual connect():s" (invoked via `-connect`, `-proxy` or `addnode`) are still reported as usual:

```
$ src/bitcoind -printtoconsole -connect=10.11.12.13
…
2018-02-28 18:33:13 connect() to 10.11.12.13:18333 failed after select(): Connection refused (111)
$ src/bitcoind -printtoconsole -proxy=10.11.12.13
…
2018-02-28 18:32:32 connect() to 10.11.12.13:9050 failed after select(): Connection refused (111)
$ src/bitcoind -printtoconsole &
$ src/bitcoin-cli addnode "10.11.12.13" onetry
…
2018-02-28 18:34:40 connect() to 10.11.12.13:18333 failed after select(): Connection refused (111)
```
@furszy
trivial: activemasternode clang-tidy cleanups
6e9b6ee
@furszy furszy self-assigned this Feb 10, 2022
@furszy furszy force-pushed the 2022_fix_dmn_operator_connection_test branch from bc4c169 to 95bb364 Compare February 10, 2022 16:35
@random-zebra random-zebra added this to the 6.0.0 milestone Feb 11, 2022
random-zebra
random-zebra approved these changes Feb 11, 2022
View reviewed changes
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 95bb364

@furszy furszy mentioned this pull request Feb 14, 2022
Merged
Fuzzbawls
Fuzzbawls approved these changes Feb 20, 2022
View reviewed changes
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 95bb364

@furszy furszy merged commit 5bc3245 into PIVX-Project:master Feb 20, 2022
furszy added a commit that referenced this pull request Feb 28, 2022
@furszy
Merge #2745: Error out at startup if there was an internal problem du…
fccfe2c 
…ring DMN init

f48176b CMasternodePayments: remove unnecessary logging (furszy)
51c6ed3 net: no need to reject incoming connections if the node is a Masternode. (furszy)
67283a8 Init: activate MN after connman initialization. (furszy)
1bba830 Init: Error out at startup if there was an internal problem during DMN init. (furszy)

Pull request description:

  Follow-up to #2742.

  1) Notify user if the Deterministic MN initialization failed for a local non-recoverable error.

  2) Moved the startup Masternode activation after the connection manager initialization.
  Without it, as the network module isn't initialized, the MN connectivity check always fails.

  3) Removed an overkill check that was rejecting incoming connections on Masternodes if the node wasn't, tier two wise, synchronized.
  The tier two sync preference for outbound connections can, and will, be achieved differently on a subsequent PR.

ACKs for top commit:
  random-zebra:
    utACK f48176b
  Fuzzbawls:
    utACK f48176b

Tree-SHA512: 491966f9a31a077da0354edd5d5370a37d88ff6b9409a21d90a130a9571a81181816f565ba430a87ad08e16f1c3499cd5712dc8b5decf32b275dab5da9b2690c
@furszy furszy deleted the 2022_fix_dmn_operator_connection_test branch August 11, 2023 03:06
panleone pushed a commit to panleone/PIVX that referenced this pull request Nov 5, 2024
@codablock @panleone
Don't be too harsh for invalid CLSIGs (PIVX-Project#2742)
924ee8f 
The local node might be the bad one actually as it might not have catched
up with the chain. In that case, LLMQs might be different for the sending
and receiving node.
Fuzzbawls added a commit that referenced this pull request Nov 9, 2024
@Fuzzbawls
Merge #2946: Dash LLMQ backports
a34f0d3 
901976a Stop tracking interested/participating nodes and send/announce to MNAUTH peers (#2798) (Alexander Block)
59dfdc2 update bestChainLockWithKnownBlock in AcceptedBlockHeader (Alessandro Rezzi)
bd0a3f8 Introduce "qsendrecsigs" to indicate that plain recovered sigs should be sent (#2783) (Alexander Block)
846fd9a Make LLMQ/InstantSend/ChainLocks code less spammy (#2781) (Alexander Block)
1a52ad0 Fix LogPrintf call in ::DoInvalidateBlock (Alessandro Rezzi)
e3c5eef Multiple fixes/refactorings for ChainLocks (#2765) (Alexander Block)
9dda5d2 Implement LLMQ based InstantSend (#2735) (Alexander Block)
149e3e3 Various small cleanups (#2761) (UdjinM6)
cc7450d Use ReleaseNodeVector and CopyNodeVector in PIVX specific code (Alessandro Rezzi)
ed463a6 Do not hold cs_vNodes in CSigSharesManager::SendMessages() for too long (#2758) (UdjinM6)
cd1809f Implement persistence for LLMQ based InstantSend (#2756) (Alexander Block)
924ee8f Don't be too harsh for invalid CLSIGs (#2742) (Alexander Block)
f255314 Fix banning when local node doesn't have the vvec (#2739) (Alexander Block)

Pull request description:

  each commit backports a different PR. The PR number is in the commit message.

  There are also 3 extra (trivial) commits added by me

ACKs for top commit: 901976a
  Duddino:
    utACK 901976a
  Liquid369:
    uTACK 901976a

Tree-SHA512: 70b53ae3fc014c23964cae3452f6c8270d96b3b375c0f172e04039ba6f3b537d56c38677e5e4005c8b810a82b447a64fc978c2d63344b9e9736ceff7db162a18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@random-zebra random-zebra random-zebra approved these changes

@Fuzzbawls Fuzzbawls Fuzzbawls approved these changes

Assignees

@furszy furszy

Labels
Bug Masternode
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@furszy @Fuzzbawls @random-zebra @UdjinM6 @practicalswift