Commit Graph

13659 Commits

Author SHA1 Message Date
Ilya Savinov
4ed838cb5d Fix MasternodeRateCheck (#1490)
* fixed an issue with MasternodeRateCheck always returns true

* additioanal fixes and refactoring (rebase)

* slightly improved CRateBuffer synchronization for newly started nodes
2017-08-23 17:22:23 +03:00
Alex Werner
ae909d0a09 Fix : Reject invalid instantsend transaction (#1583) 2017-08-23 17:21:54 +03:00
UdjinM6
f786ce6aba fix instantsendtoaddress param convertion (#1585)
`subtractfeefromamount` is bool but `true`/`false` are not recognized in rpc atm. This should fix it.
2017-08-23 17:21:33 +03:00
Alexander Block
ccee103a0e Backport "assumed valid blocks" feature from Bitcoin 0.13 (#1582)
* IBD check uses minimumchain work instead of checkpoints.

This introduces a 'minimum chain work' chainparam which is intended
 to be the known amount of work in the chain for the network at the
 time of software release.  If you don't have this much work, you're
 not yet caught up.

This is used instead of the count of blocks test from checkpoints.

This criteria is trivial to keep updated as there is no element of
subjectivity, trust, or position dependence to it. It is also a more
reliable metric of sync status than a block count.

* Remove GetTotalBlocksEstimate and checkpoint tests that test nothing.

GetTotalBlocksEstimate is no longer used and it was the only thing
 the checkpoint tests were testing.

Since checkpoints are on their way out it makes more sense to remove
 the test file than to cook up a new pointless test.

# Conflicts:
#	src/Makefile.test.include
#	src/test/Checkpoints_tests.cpp

* IsInitialBlockDownload no longer uses header-only timestamps.

This avoids a corner case (mostly visible on testnet) where bogus
 headers can keep nodes in IsInitialBlockDownload.

* Delay parallel block download until chain has sufficient work

nMinimumChainWork is an anti-DoS threshold; wait until we have a proposed
tip with more work than that before downloading blocks towards that tip.

* Add timeout for headers sync

At startup, we choose one peer to serve us the headers chain, until
our best header is close to caught up.  Disconnect this peer if more
than 15 minutes + 1ms/expected_header passes and our best header
is still more than 1 day away from current time.

* Introduce assumevalid setting to skip presumed valid scripts.

This disentangles the script validation skipping from checkpoints.

A new option is introduced "assumevalid" which specifies a block whos
 ancestors we assume all have valid scriptsigs and so we do not check
 them when they are also burried under the best header by two weeks
 worth of work.

Unlike checkpoints this has no influence on consensus unless you set
 it to a block with an invalid history.  Because of this it can be
 easily be updated without risk of influencing the network consensus.

This results in a massive IBD speedup.

This approach was independently recommended by Peter Todd and Luke-Jr
 since POW based signature skipping (see PR#9180) does not have the
 verifiable properties of a specific hash and may create bad incentives.

The downside is that, like checkpoints, the defaults bitrot and older
 releases will sync slower.  On the plus side users can provide their
 own value here, and if they set it to something crazy all that will
 happen is more time will be spend validating signatures.

Checkblocks and checklevel are also moved to the hidden debug options:
 Especially now that checkblocks has a low default there is little need
 to change these settings, and users frequently misunderstand them as
 influencing security or IBD speed.  By hiding them we offset the
 space added by this new option.

* Add consensusParams to FindNextBlocksToDownload

* Adjust check in headers timeout logic to align with 144 blocks in Dash
2017-08-23 17:21:08 +03:00
Oleg Girko
2472999da0 Backport Bitcoin PR#9441: Net: Massive speedup. Net locks overhaul (#1586)
* net: fix typo causing the wrong receive buffer size

Surprisingly this hasn't been causing me any issues while testing, probably
because it requires lots of large blocks to be flying around.

Send/Recv corks need tests!

* net: make vRecvMsg a list so that we can use splice()

* net: make GetReceiveFloodSize public

This will be needed so that the message processor can cork incoming messages

* net: only disconnect if fDisconnect has been set

These conditions are problematic to check without locking, and we shouldn't be
relying on the refcount to disconnect.

* net: wait until the node is destroyed to delete its recv buffer

when vRecvMsg becomes a private buffer, it won't make sense to allow other
threads to mess with it anymore.

* net: set message deserialization version when it's actually time to deserialize

We'll soon no longer have access to vRecvMsg, and this is more intuitive anyway.

* net: handle message accounting in ReceiveMsgBytes

This allows locking to be pushed down to only where it's needed

Also reuse the current time rather than checking multiple times.

* net: record bytes written before notifying the message processor

* net: Add a simple function for waking the message handler

This may be used publicly in the future

* net: remove useless comments

* net: remove redundant max sendbuffer size check

This is left-over from before there was proper accounting. Hitting 2x the
sendbuffer size should not be possible.

* net: rework the way that the messagehandler sleeps

In order to sleep accurately, the message handler needs to know if _any_ node
has more processing that it should do before the entire thread sleeps.

Rather than returning a value that represents whether ProcessMessages
encountered a message that should trigger a disconnnect, interpret the return
value as whether or not that node has more work to do.

Also, use a global fProcessWake value that can be set by other threads,
which takes precedence (for one cycle) over the messagehandler's decision.

Note that the previous behavior was to only process one message per loop
(except in the case of a bad checksum or invalid header). That was changed in
PR #3180.

The only change here in that regard is that the current node now falls to the
back of the processing queue for the bad checksum/invalid header cases.

* net: add a new message queue for the message processor

This separates the storage of messages from the net and queued messages for
processing, allowing the locks to be split.

* net: add a flag to indicate when a node's process queue is full

Messages are dumped very quickly from the socket handler to the processor, so
it's the depth of the processing queue that's interesting.

The socket handler checks the process queue's size during the brief message
hand-off and pauses if necessary, and the processor possibly unpauses each time
a message is popped off of its queue.

* net: add a flag to indicate when a node's send buffer is full

Similar to the recv flag, but this one indicates whether or not the net's send
buffer is full.

The socket handler checks the send queue when a new message is added and pauses
if necessary, and possibly unpauses after each message is drained from its buffer.

* net: remove cs_vRecvMsg

vRecvMsg is now only touched by the socket handler thread.

The accounting vars (nRecvBytes/nLastRecv/mapRecvBytesPerMsgCmd) are also
only used by the socket handler thread, with the exception of queries from
rpc/gui. These accesses are not threadsafe, but they never were. This needs to
be addressed separately.

Also, update comment describing data flow
2017-08-23 17:20:43 +03:00
Oleg Girko
b9c67258ba Backport Bitcoin PR#9609: net: fix remaining net assertions (#1575) + Dashify
* Dont deserialize nVersion into CNode, should fix #9212

* net: deserialize the entire version message locally

This avoids having some vars set if the version negotiation fails.

Also copy it all into CNode at the same site. nVersion and
fSuccessfullyConnected are set last, as they are the gates for the other vars.
Make them atomic for that reason.

* net: don't run callbacks on nodes that haven't completed the version handshake

Since ForEach* are can be used to send messages to  all nodes, the caller may
end up sending a message before the version handshake is complete. To limit
this, filter out these nodes. While we're at it, may as well filter out
disconnected nodes as well.

Delete unused methods rather than updating them.

* net: Disallow sending messages until the version handshake is complete

This is a change in behavior, though it's much more sane now than before.

* net: log an error rather than asserting if send version is misused

Also cleaned up the comments and moved from the header to the .cpp so that
logging headers aren't needed from net.h

* Implement conditions for ForEachNode() and ForNode() methods of CConnman.

A change making ForEachNode() and ForNode() methods ignore nodes that
have not completed initial handshake have been backported from Bitcoin.
Unfortunately, some Dash-specific code needs to iterate over all nodes.

This change introduces additional condition argument to these methods.
This argument is a functional object that should return true for nodes
that should be taken into account, not ignored.

Two functional objects are provided in CConnman namespace:
* FullyConnectedOnly returns true for nodes that have handshake completed,
* AllNodes returns true for all nodes.

Overloads for ForEachNode() and ForNode() methods without condition argument
are left for compatibility with non-Dash-specific code.
They use FullyConnectedOnly functional object for condition.

Signed-off-by: Oleg Girko <ol@infoserver.lv>

* Iterate over all nodes in Dash-specific code using AllNodes condition.

Use AllNodes functional object as newly introduced condition argument for
ForEachNode() and ForNode() methods of CConnman to iterate over all nodes
where needed in Dash-specific code.

Signed-off-by: Oleg Girko <ol@infoserver.lv>
2017-08-17 21:37:22 +03:00
Will Wray
8b5f47e686 Masternode classes: Remove repeated/un-needed code and data (#1572)
* Remove un-needed #includes (what is the policy?)

Data was duplicated in masternode_info_t and CMasternode classes:

* CMasternode is changed to inherit from masternode_info_t
  so the data members are inherited rather than repeated
 (also inherits unrepeated nTimeLastPing and fInfoValid members;
  this slight intrusiveness made up for in simplicity).

* Use in-class member initializers (C++11) for defaults,
  so only non-default initializers are required in the lists.
  Allows to shorten repetitous constructor initializer lists.
  This makes checking for uninitialized data simpler.

* Default constructors are defined as "= default;" if possible.

* masternode_info_t is changed to behave like an aggregate
 (but requires over-complicated constructors until c++14).
  There are pros and cons here - aggregate initialization
  is convenient but implicit).

* Removed user-defined swap functions.
  They appear to only be used in operator= definitions,
  using the copy-in,swap-and-return idiom:

* Default operator=, where possible.

* Move in class `friend bool operator==` out-of-class.
2017-08-11 21:52:06 +03:00
Ilya Savinov
4a9fbca080 Fix potential deadlock in CInstantSend::UpdateLockedTransaction (#1571)
* cs_wallet -> cs_instantsend:

getinfo(...) - LOCK2(cs_main, pwalletMain ? &pwalletMain->cs_wallet : NULL)
  CAmount CWallet::GetBalance() const - LOCK2(cs_main, cs_wallet)
    CWalletTx::IsTrusted()
      GetDepthInMainChain()
        CInstantSend::IsLockedInstantSendTransaction(...) - LOCK(cs_instantsend)

cs_wallet -> cs_instantsend:

CWallet::ReacceptWalletTransactions() - LOCK2(cs_main, cs_wallet)
  GetDepthInMainChain()
    CInstantSend::IsLockedInstantSendTransaction(...) - LOCK(cs_instantsend)

cs_instantsend -> cs_wallet:

void CInstantSend::UpdateLockedTransaction(...) - LOCK(cs_instantsend)
  CWallet::UpdatedTransaction(...) - LOCK(cs_wallet)

* AssertLockHeld added
2017-08-11 21:51:44 +03:00
chaeplin
0fc13434b4 limit UpdatedBlockTip in IBD (#1570) 2017-08-11 21:51:24 +03:00
UdjinM6
c608bbec1d Pass reference when calling HasPayeeWithVotes (#1569) 2017-08-11 21:50:57 +03:00
UdjinM6
105713c10a Sync overhaul (#1564)
* Change sync process:
- IsBlockchainSynced(): drop CheckNodeHeight() and all complicated code, use fInitialDownload in UpdatedBlockTip() to switch initial states
- ProcessTick(): detect sleep mode like it was in IsBlockchainSynced(), not by number of masternodes

* Changes for sync in governance:
- do not keep sync alive on ConfirmInventoryRequest()
- skip some governance actions until we are synced to some level

* do not run CMasternodeMan::UpdateLastPaid() until winners list is synced

* start syncing mn list on the same node right after requesting sporks

* replace nTimeLast<Asset> with the unified nTimeLastBumped, bump on UpdatedBlockTip

* fix comments and LogPrintf-s

* remove excessive MASTERNODE_SYNC_IBD

* a bit more descriptive BumpAssetLastTime in few cases
2017-08-09 19:07:03 +03:00
Oleg Girko
42c784dc7a Backport Bitcoin PR#9289: net: drop boost::thread_group (#1568)
* net: a few small cleanups before replacing boost threads

- Drop the interruption point directly after the pnode allocation. This would
    be leaky if hit.
- Rearrange thread creation so that the socket handler comes first

* net: add CThreadInterrupt and InterruptibleSleep

* net: make net interruptible

Also now that net threads are interruptible, switch them to use std
threads/binds/mutexes/condvars.

* net: make net processing interruptible

* net: remove thread_interrupted catch

This is now a std::thread, so there's no hope of catching a boost interruption
point.

* net: make proxy receives interruptible

* net: misc header cleanups
2017-08-09 19:06:31 +03:00
Oleg Girko
df6d458b85 Backport Bitcoin PR#9260: Mrs Peacock in The Library with The Candlestick (killed main.{h,cpp}) (#1566)
* Remove orphan state wipe from UnloadBlockIndex.

As orphan state is now "network state", like in
d6ea737be1,

UnloadBlockIndex is only used during init if we end up reindexing
to clear our block state so that we can start over. However, at
that time no connections have been brought up as CConnman hasn't
been started yet, so all of the network processing state logic is
empty when its called.

* Move network-msg-processing code out of main to its own file

* Rename the remaining main.{h,cpp} to validation.{h,cpp}
2017-08-09 03:19:06 +03:00
Oleg Girko
bcf5455bfd Backport Bitcoin PR#8822: net: Consistent checksum handling (#1565)
* net: Consistent checksum handling

In principle, the checksums of P2P packets are simply 4-byte blobs which
are the first four bytes of SHA256(SHA256(payload)).

Currently they are handled as little-endian 32-bit integers half of the
time, as blobs the other half, sometimes copying the one to the other,
resulting in somewhat confused code.

This PR changes the handling to be consistent both at packet creation
and receiving, making it (I think) easier to understand.

* net: Hardcode protocol sizes and use fixed-size types

The P2P network uses a fixed protocol, these sizes shouldn't change
based on what happens to be the architecture.
2017-08-08 20:57:36 +03:00
UdjinM6
87707c0127 fix CDSNotificationInterface::UpdatedBlockTip signature (#1562)
to match the one in CValidationInterface
2017-08-04 20:43:48 +03:00
Oleg Girko
415085c732 Backport Bitcoin PR#9183: Final Preparation for main.cpp Split (#1561)
* Expose AcceptBlockHeader through main.h

* Split ::HEADERS processing into two separate cs_main locks

This will allow NotifyHeaderTip to be called from an
AcceptBlockHeader wrapper function without holding cs_main.

* Use exposed ProcessNewBlockHeaders from ProcessMessages
2017-08-03 01:19:34 +03:00
Oleg Girko
b4b3431458 Backport Bitcoin PR#9075: Decouple peer-processing-logic from block-connection-logic (#3) (#1560)
* Remove pfrom parameter from ProcessNewBlock

This further decouples ProcessNewBlock from networking/peer logic.

* Replace CValidationState param in ProcessNewBlock with BlockChecked
2017-08-02 21:35:04 +03:00
Oleg Girko
e7e106e228 Backport Bitcoin PR#8969: Decouple peer-processing-logic from block-connection-logic (#2) (#1558)
* Move MarkBlockAsReceived out of ProcessNewMessage

* Remove network state wipe from UnloadBlockIndex.

UnloadBlockIndex is only used during init if we end up reindexing
to clear our block state so that we can start over. However, at
that time no connections have been brought up as CConnman hasn't
been started yet, so all of the network processing state logic is
empty when its called.

Additionally, the initialization of the recentRejects set is moved
to InitPeerLogic.

* Move all calls to CheckBlockIndex out of net-processing logic

This will result in many more calls to CheckBlockIndex when
connecting a list of headers (eg in ::HEADERS messages processing)
but its only enabled in debug mode, and that should mostly just be
during IBD, so it should be OK.

* Move FlushStateToDisk call out of ProcessMessages::TX into ATMP

* Move nTimeBestReceived updating into net processing code
2017-08-01 18:11:32 +03:00
Oleg Girko
a3c8cb20df Backport Bitcoin PR#8865: Decouple peer-processing-logic from block-connection-logic (#1556)
* Make validationinterface.UpdatedBlockTip more verbose

In anticipation of making all the callbacks out of block processing
flow through it. Note that vHashes will always have something in it
since pindexFork != pindexNewTip.

* Remove duplicate nBlocksEstimate cmp (we already checked IsIBD())

* Remove CConnman parameter from ProcessNewBlock/ActivateBestChain

* Remove SyncWithWallets wrapper function

* Move net-processing logic definitions together in main.h

* Use CValidationInterface from chain logic to notify peer logic

This adds a new CValidationInterface subclass, defined in main.h,
to receive notifications of UpdatedBlockTip and use that to push
blocks to peers, instead of doing it directly from
ActivateBestChain.

* Always call UpdatedBlockTip, even if blocks were only disconnected

* Use BlockChecked signal to send reject messages from mapBlockSource
2017-07-28 17:10:10 +03:00
Oleg Girko
cc4db34f4d net: only delete CConnman if it's been created (#1555)
In the case of (for example) an already-running bitcoind, the shutdown sequence
begins before CConnman has been created, leading to a null-pointer dereference
when g_connman->Stop() is called.

Instead, Just let the CConnman dtor take care of stopping.
2017-07-28 01:39:53 +03:00
Oleg Girko
b621cfb5fb Backport Bitcoin PR#8708: net: have CConnman handle message sending (#1553)
* serialization: teach serializers variadics

Also add a variadic CDataStream ctor for ease-of-use.

* connman is in charge of pushing messages

The changes here are dense and subtle, but hopefully all is more explicit
than before.

- CConnman is now in charge of sending data rather than the nodes themselves.
  This is necessary because many decisions need to be made with all nodes in
  mind, and a model that requires the nodes calling up to their manager quickly
  turns to spaghetti.

- The per-node-serializer (ssSend) has been replaced with a (quasi-)const
  send-version. Since the send version for serialization can only change once
  per connection, we now explicitly tag messages with INIT_PROTO_VERSION if
  they are sent before the handshake. With this done, there's no need to lock
  for access to nSendVersion.

  Also, a new stream is used for each message, so there's no need to lock
  during the serialization process.

- This takes care of accounting for optimistic sends, so the
  nOptimisticBytesWritten hack can be removed.

- -dropmessagestest and -fuzzmessagestest have not been preserved, as I suspect
  they haven't been used in years.

* net: switch all callers to connman for pushing messages

Drop all of the old stuff.

* drop the optimistic write counter hack

This is now handled properly in realtime.

* net: remove now-unused ssSend and Fuzz

* net: construct CNodeStates in place

* net: handle version push in InitializeNode
2017-07-27 17:28:05 +03:00
UdjinM6
8b7dffbb64 Update startup shortcuts (#1551) 2017-07-25 13:58:27 +03:00
UdjinM6
4df8a20f96 Fix windows installer script, should handle dash: uri correctly now (#1550) 2017-07-25 13:58:08 +03:00
UdjinM6
9694658cd3 Make sure mixing messages are relayed/accepted properly (#1547)
* make sure addr of mixing mn matches peer's addr exactly (including port)

* Store addr of every mixing client and relay mixing mesasges to them only
2017-07-25 13:57:26 +03:00
Nathan Marley
a0851494db add masternodelist pubkey to rpc (#1549)
* add masternodelist pubkey to rpc

* add method in alphabetical order
2017-07-22 23:33:42 +03:00
UdjinM6
7707c07897 backport 9008: Remove assert(nMaxInbound > 0) (#1548) 2017-07-22 22:37:10 +03:00
Oleg Girko
82851b439c Backport Bitcoin PR#8049: Expose information on whether transaction relay is enabled in getnetwork (#1545)
* net: Add fRelayTxes flag

Add a fRelayTxes to keep track of the relay transaction flag
we send to other peers.

* rpc: Add `relaytxes` flag to `getnetworkinfo`

Re-work of PR #7841 by dragongem45.
Closes #7771.
2017-07-21 21:31:47 +03:00
Ilya Savinov
4942884c71 fix potential deadlock in CMasternodeMan::CheckMnbAndUpdateMasternodeList (#1543)
governance.cs -> mnodeman.cs:
CGovernanceManager::ProcessMessage - LOCK2(cs_main, governance.cs) -> AddGovernanceObject -> UpdateSentinelVariables -> CountEnabled - LOCK(mnodeman.cs)

mnodeman.cs -> governance.cs:
CheckMnbAndUpdateMasternodeList - LOCK2(cs_main, mnodeman.cs) -> activeMasternode.ManageState -> ManageStateLocal -> NotifyMasternodeUpdates -> СheckMasternodeOrphanObjects - LOCK2(cs_main, governance.cs)
... -> NotifyMasternodeUpdates -> CheckMasternodeOrphanVotes - LOCK2(cs_main, governance.cs)
... -> NotifyMasternodeUpdates -> UpdateCachesAndClean - LOCK2(cs_main, governance.cs)
2017-07-21 18:21:09 +03:00
Ilya Savinov
1c4e2946af fix potential deadlock in CGovernanceManager::ProcessVote (#1541)
govman.cs -> cs_vSend:
CGovernanceManager::ProcessVote - LOCK(govman.cs) -> RequestGovernanceObject -> PushMessage -> BeginMessage - LOCK(cs_vSend)

cs_vSend -> govman.cs:
ThreadMessageHandler -> TRY_LOCK(pnode->cs_vSend, lockSend) -> SendMessages -> AlreadyHave -> ConfirmInventoryRequest -> LOCK(govman.cs)
2017-07-21 12:47:38 +03:00
chaeplin
f47f0daf9a add 6 to strAllowedChars (#1542) 2017-07-21 12:36:34 +03:00
Oleg Girko
a9d771e497 Backport Bitcoin PR#8085: p2p: Begin encapsulation (#1537)
* net: move CBanDB and CAddrDB out of net.h/cpp

This will eventually solve a circular dependency

* net: Create CConnman to encapsulate p2p connections

* net: Move socket binding into CConnman

* net: move OpenNetworkConnection into CConnman

* net: move ban and addrman functions into CConnman

* net: Add oneshot functions to CConnman

* net: move added node functions to CConnman

* net: Add most functions needed for vNodes to CConnman

* net: handle nodesignals in CConnman

* net: Pass CConnection to wallet rather than using the global

* net: Add rpc error for missing/disabled p2p functionality

* net: Pass CConnman around as needed

* gui: add NodeID to the peer table

* net: create generic functor accessors and move vNodes to CConnman

* net: move whitelist functions into CConnman

* net: move nLastNodeId to CConnman

* net: move nLocalHostNonce to CConnman

This behavior seems to have been quite racy and broken.

Move nLocalHostNonce into CNode, and check received nonces against all
non-fully-connected nodes. If there's a match, assume we've connected
to ourself.

* net: move messageHandlerCondition to CConnman

* net: move send/recv statistics to CConnman

* net: move SendBufferSize/ReceiveFloodSize to CConnman

* net: move nLocalServices/nRelevantServices to CConnman

These are in-turn passed to CNode at connection time. This allows us to offer
different services to different peers (or test the effects of doing so).

* net: move semOutbound and semMasternodeOutbound to CConnman

* net: SocketSendData returns written size

* net: move max/max-outbound to CConnman

* net: Pass best block known height into CConnman

CConnman then passes the current best height into CNode at creation time.

This way CConnman/CNode have no dependency on main for height, and the signals
only move in one direction.

This also helps to prevent identity leakage a tiny bit. Before this change, an
attacker could theoretically make 2 connections on different interfaces. They
would connect fully on one, and only establish the initial connection on the
other. Once they receive a new block, they would relay it to your first
connection, and immediately commence the version handshake on the second. Since
the new block height is reflected immediately, they could attempt to learn
whether the two connections were correlated.

This is, of course, incredibly unlikely to work due to the small timings
involved and receipt from other senders. But it doesn't hurt to lock-in
nBestHeight at the time of connection, rather than letting the remote choose
the time.

* net: pass CClientUIInterface into CConnman

* net: Drop StartNode/StopNode and use CConnman directly

* net: Introduce CConnection::Options to avoid passing so many params

* net: add nSendBufferMaxSize/nReceiveFloodSize to CConnection::Options

* net: move vNodesDisconnected into CConnman

* Made the ForEachNode* functions in src/net.cpp more pragmatic and self documenting

* Convert ForEachNode* functions to take a templated function argument rather than a std::function to eliminate std::function overhead

* net: move MAX_FEELER_CONNECTIONS into connman
2017-07-21 12:35:19 +03:00
Ilya Savinov
15958c594c fix potential deadlock (PR#1536 fix) (#1538)
mempool.cs -> mnodeman.cs:
`CreateNewBlock - LOCK2(cs_main, mempool.cs) -> FillBlockPayments -> FillBlockPayee -> GetNextMasternodeInQueueForPayment - LOCK(mnodeman.cs)`

mnodeman.cs -> mempool.cs
`CheckMnbAndUpdateMasternodeList - LOCK2(cs_main, mnodeman.cs) -> CheckOutpoint -> GetTransaction -> lookup - LOCK(mempool.cs)`
2017-07-20 13:48:30 +03:00
UdjinM6
0f05e25c71 fix a bug in CommitFinalTransaction (#1540) 2017-07-19 21:49:48 +02:00
Ilya Savinov
6cb3fddcc1 fixed potential deadlock in CSuperblockManager::IsSuperblockTriggered (#1536)
governance.cs -> mempool.cs:
`CGovernanceManager::ProcessMessage - LOCK2(cs_main, governance.cs) ->  GetTransaction ->  lookup - LOCK(mempool.cs)`

mempool.cs -> governance.cs:
`CreateNewBlock - LOCK2(cs_main, mempool.cs)  -> FillBlockPayments ->  IsSuperblockTriggered - LOCK(governance.cs)`
2017-07-18 19:25:02 +03:00
Oleg Girko
290fb3b57e Backport Bitcoin PR#7942: locking for Misbehave() and other cs_main locking fixes (#1535)
* lock cs_main for chainActive

ActivateBestChain uses chainActive after releasing the lock; reorder operations
to move all access to synchronized object into existing LOCK(cs_main) block.

* lock cs_main for State/Misbehaving

ProcessMessage calls State(...) and Misbehaving(...) without holding the
required lock; add LOCK(cs_main) blocks.
2017-07-17 20:49:17 +03:00
gladcow
8075370d1e change invalid version string constant (#1532) 2017-07-17 17:38:32 +03:00
Ilya Savinov
e0d6c5b5a2 fixed potential deadlock in CMasternodePing::SimpleCheck (#1534) 2017-07-17 16:59:04 +03:00
UdjinM6
9e9df28203 do not use masternode connections in feeler logic (#1533) 2017-07-17 16:54:35 +03:00
Oleg Girko
d934ffb2ff Added feeler connections increasing good addrs in the tried table. (#1530)
Tests if addresses are online or offline by briefly connecting to them. These short lived connections are referred to as feeler connections. Feeler connections are designed to increase the number of fresh online addresses in tried by selecting and connecting to addresses in new. One feeler connection is attempted on average once every two minutes.

This change was suggested as Countermeasure 4 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.
2017-07-17 13:39:12 +03:00
UdjinM6
27b6f36338 fix deadlock (#1531) 2017-07-17 10:06:15 +02:00
Oleg Girko
aa32f1dc97 Remove bad chain alert partition check (#1529)
As per meeting 2016-03-31
https://bitcoincore.org/en/meetings/2016/03/31/#bad-chain-alerts

The partition checker was producing huge number of false-positives
and was disabled in 0.12.1 on the understanding it would either be
fixed in 0.13 or removed entirely from master if not.

*** Dash specific note: ***
This check was disabled in Dash already.
2017-07-16 23:17:04 +03:00
Oleg Girko
9ce2b966cd Backport Bitcoin PR#8113: Rework addnode behaviour (#1525)
* Rework addnode behaviour

* Use CNode::addeName to track whether a connection to a name is already open
  * A new connection to a previously-connected by-name addednode is only opened when
    the previous one closes (even if the name starts resolving to something else)
  * At most one connection is opened per addednode (even if the name resolves to multiple)
* Unify the code between ThreadOpenAddedNodeConnections and getaddednodeinfo
  * Information about open connections is always returned, and the dns argument becomes a dummy
  * An IP address and inbound/outbound is only reported for the (at most 1) open connection

* Prevent duplicate connections where one is by name and another by ip

* Randomize name lookup result in ConnectSocketByName
2017-07-14 19:58:57 +03:00
UdjinM6
1b1d52ac3e workaround for travis (#1526) 2017-07-14 17:19:07 +03:00
Ilya Savinov
916af52c0a Fix vulnerability with mapMasternodeOrphanObjects (#1512)
* fix vulnerability with mapMasternodeOrphanObjects

The vulnerability is that a malicious node can send a lot of NetMsgType::MNGOVERNANCEOBJECT messages which refer to many arbitrary MN's. In this case, mapMasternodeOrphanObjects will grow unrestrictedly.

* MN collateral moved to governance-object.cpp; ban score applied to misbehaving nodes

* recursive locks removed

* check for the mn collateral code segregated to a separate function

* CheckCollateral implementation moved to cpp
2017-07-13 12:38:00 +03:00
Oleg Girko
eea78d45ed Revert "fixed regtest+ds issues" (#1524)
This reverts commit 1f828f45ec.

The commit being reverted changed FindNode(const CService& addr)
to make no difference between nodes with the same IP address,
but different ports, but only for regtest network.
As functional tests run several nodes on different ports or the same
IP address (127.0.0.1), this eventually started breaking functional tests.
The only use for regtest network I know is for functional tests,
so it's time to revert that commit.
2017-07-13 09:07:50 +03:00
chaeplin
f9dd40888e include atomic (#1523) 2017-07-12 23:08:18 +03:00
Ilya Savinov
f7aa81586f Fix issues with mapSeenGovernanceObjects (#1511)
* fix issues with mapSeenGovernanceObjects

Removed seen-governance-objects optimization except for deleted objects. Otherwise some nodes can permanently lost proposals if they received them too early.
Beside of that there is a vulnerability with seen-governance-objects mechanism if malicious node send us a lot of invalid governance objects.

* mapSeenGovernanceObjects renamed to mapErasedGovernanceObjects

* current fixes

* use int64_t for expiration timestamp
2017-07-12 23:08:06 +03:00
Oleg Girko
7b5556a294 Backport Bitcoin PR#8084: Add recently accepted blocks and txn to AttemptToEvictConnection (#1522)
* Add recently accepted blocks and txn to AttemptToEvictConnection.

This protects any not-already-protected peers who were the most
 recent four to relay transactions and most recent four to send
 blocks to us.

* Allow disconnecting a netgroup with only one member in eviction.

With the latest additions there are enough protective measures that
 we can take the training wheels off.
2017-07-12 14:13:38 +03:00
Oleg Girko
5a1961e5e3 Backport Bitcoin PR#7906: net: prerequisites for p2p encapsulation changes (#1521)
* net: don't import std namespace

This file is about to be broken up into chunks and moved around. Drop the
namespace now rather than requiring other files to use it.

* net: remove unused set

* net: use the exposed GetNodeSignals() rather than g_signals directly

* net: make Ban/Unban/ClearBan functionality consistent

- Ban/Unban/ClearBan call uiInterface.BannedListChanged() as necessary
- Ban/Unban/ClearBan sync to disk if the operation is user-invoked
- Mark node for disconnection automatically when banning
- Lock cs_vNodes while setting disconnected
- Don't spin in a tight loop while setting disconnected

* net: No need to export DumpBanlist
2017-07-12 04:20:12 +03:00
Oleg Girko
9a8a290b87 fix race that could fail to persist a ban (#1518)
DumpBanList currently does this:
  - with lock: take a copy of the banmap
  - perform I/O (write out the banmap)
  - with lock: mark the banmap non-dirty
If a new ban is added during the I/O operation, it may never be persisted to
disk.

Reorder operations so that the data to be persisted cannot be older than the
time at which the banmap was marked non-dirty.
2017-07-11 09:15:14 +03:00