Skip to content

Commit

Permalink
Merge pull request #251 from ZencashOfficial/timeupdate
Browse files Browse the repository at this point in the history
  • Loading branch information
cronicc committed May 15, 2020
2 parents b1d779e + 28cc876 commit e4d88b2
Show file tree
Hide file tree
Showing 30 changed files with 446 additions and 248 deletions.
16 changes: 16 additions & 0 deletions doc/release-notes.md
Expand Up @@ -4,3 +4,19 @@ release-notes at release time)
Notable changes
===============

Removal of time adjustment
-------------------------------------------------------------

Prior to v2.0.21, `zend` would adjust the local time that it used by up
to 70 minutes, according to a median of the times sent by the first 200 peers
to connect to it. This mechanism was inherently insecure, since an adversary
making multiple connections to the node could effectively control its time
within that +/- 70 minute window (this is called a "timejacking attack").

In the v2.0.21 release, in addition to other mitigations for timejacking attacks,
as a simplification the time adjustment code has now been completely removed.
Node operators should instead simply ensure that local time is set
reasonably accurately.

If it appears that the node has a significantly different time than its peers,
a warning will still be logged and indicated on the metrics screen if enabled.
16 changes: 12 additions & 4 deletions qa/rpc-tests/test_framework/util.py
Expand Up @@ -115,7 +115,11 @@ def initialize_chain(test_dir):
4 wallets.
zend and zen-cli must be in search path.
"""

if os.path.isdir(os.path.join("cache", "node0")):
if os.stat("cache").st_mtime + (60 * 60) < time.time():
print("initialize_chain(): Removing stale cache")
shutil.rmtree("cache")

if not os.path.isdir(os.path.join("cache", "node0")):
devnull = open("/dev/null", "w+")
# Create cache directories, run bitcoinds:
Expand Down Expand Up @@ -145,16 +149,20 @@ def initialize_chain(test_dir):
# gets 25 mature blocks and 25 immature.
# blocks are created with timestamps 10 minutes apart, starting
# at Fri, 12 May 2017 00:15:50 GMT (genesis block time)
block_time = 1494548150
# block_time = 1494548150
block_time = int(time.time()) - (200 * 150) # 200 is number of blocks and 150 is blocktime target spacing 150 / 60 = 2.5 min
for i in range(2):
for peer in range(4):
for j in range(25):
set_node_times(rpcs, block_time)
rpcs[peer].generate(1)
block_time += 10*60
# block_time += 10*60 # this was BTC target spacing ??
block_time += 150 # ZEN blocktime target spacing
# Must sync before next peer starts generating blocks
sync_blocks(rpcs)

# Check that local time isn't going backwards
assert_greater_than(time.time() + 1, block_time)

# Shut them down, and clean up cache directories:
stop_nodes(rpcs)
wait_bitcoinds()
Expand Down
1 change: 1 addition & 0 deletions src/Makefile.am
Expand Up @@ -589,6 +589,7 @@ libzencash_a_SOURCES = \
zen/forks/fork3_communityfundandrpfixfork.cpp\
zen/forks/fork4_nulltransactionfork.cpp\
zen/forks/fork5_shieldfork.cpp\
zen/forks/fork6_timeblockfork.cpp\
zen/tlsmanager.cpp\
zen/delay.cpp

Expand Down
9 changes: 5 additions & 4 deletions src/Makefile.gtest.include
Expand Up @@ -20,11 +20,13 @@ zen_gtest_SOURCES += \
endif
zen_gtest_SOURCES += \
gtest/test_tautology.cpp \
gtest/test_checkblock.cpp \
gtest/test_deprecation.cpp \
gtest/test_equihash.cpp \
gtest/test_httprpc.cpp \
gtest/test_joinsplit.cpp \
gtest/test_keystore.cpp \
gtest/test_libzcash_utils.cpp \
gtest/test_noteencryption.cpp \
gtest/test_mempool.cpp \
gtest/test_merkletree.cpp \
Expand All @@ -34,15 +36,14 @@ zen_gtest_SOURCES += \
gtest/test_random.cpp \
gtest/test_rpc.cpp \
gtest/test_getblocktemplate.cpp \
gtest/test_timedata.cpp \
gtest/test_transaction.cpp \
gtest/test_txid.cpp \
gtest/test_validation.cpp \
gtest/test_circuit.cpp \
gtest/test_txid.cpp \
gtest/test_libzcash_utils.cpp \
gtest/test_proofs.cpp \
gtest/test_paymentdisclosure.cpp \
gtest/test_checkblock.cpp \
gtest/test_relayforks.cpp
gtest/test_relayforks.cpp

if ENABLE_WALLET
zen_gtest_SOURCES += \
Expand Down
1 change: 0 additions & 1 deletion src/Makefile.test.include
Expand Up @@ -84,7 +84,6 @@ BITCOIN_TESTS =\
test/skiplist_tests.cpp \
test/test_bitcoin.cpp \
test/test_bitcoin.h \
test/timedata_tests.cpp \
test/torcontrol_tests.cpp \
test/transaction_tests.cpp \
test/uint256_tests.cpp \
Expand Down
2 changes: 1 addition & 1 deletion src/addrman.cpp
Expand Up @@ -253,7 +253,7 @@ bool CAddrMan::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimeP

if (pinfo) {
// periodically update nTime
bool fCurrentlyOnline = (GetAdjustedTime() - addr.nTime < 24 * 60 * 60);
bool fCurrentlyOnline = (GetTime() - addr.nTime < 24 * 60 * 60);
int64_t nUpdateInterval = (fCurrentlyOnline ? 60 * 60 : 24 * 60 * 60);
if (addr.nTime && (!pinfo->nTime || pinfo->nTime < addr.nTime - nUpdateInterval - nTimePenalty))
pinfo->nTime = std::max((int64_t)0, addr.nTime - nTimePenalty);
Expand Down
10 changes: 5 additions & 5 deletions src/addrman.h
Expand Up @@ -97,10 +97,10 @@ class CAddrInfo : public CAddress
int GetBucketPosition(const uint256 &nKey, bool fNew, int nBucket) const;

//! Determine whether the statistics about this entry are bad enough so that it can just be deleted
bool IsTerrible(int64_t nNow = GetAdjustedTime()) const;
bool IsTerrible(int64_t nNow = GetTime()) const;

//! Calculate the relative chance this entry should be given when selecting nodes to connect to
double GetChance(int64_t nNow = GetAdjustedTime()) const;
double GetChance(int64_t nNow = GetTime()) const;

};

Expand Down Expand Up @@ -520,7 +520,7 @@ class CAddrMan
}

//! Mark an entry as accessible.
void Good(const CService &addr, int64_t nTime = GetAdjustedTime())
void Good(const CService &addr, int64_t nTime = GetTime())
{
{
LOCK(cs);
Expand All @@ -531,7 +531,7 @@ class CAddrMan
}

//! Mark an entry as connection attempted to.
void Attempt(const CService &addr, int64_t nTime = GetAdjustedTime())
void Attempt(const CService &addr, int64_t nTime = GetTime())
{
{
LOCK(cs);
Expand Down Expand Up @@ -570,7 +570,7 @@ class CAddrMan
}

//! Mark an entry as currently-connected-to.
void Connected(const CService &addr, int64_t nTime = GetAdjustedTime())
void Connected(const CService &addr, int64_t nTime = GetTime())
{
{
LOCK(cs);
Expand Down
4 changes: 2 additions & 2 deletions src/alert.cpp
Expand Up @@ -102,7 +102,7 @@ uint256 CAlert::GetHash() const

bool CAlert::IsInEffect() const
{
return (GetAdjustedTime() < nExpiration);
return (GetTime() < nExpiration);
}

bool CAlert::Cancels(const CAlert& alert) const
Expand Down Expand Up @@ -137,7 +137,7 @@ bool CAlert::RelayTo(CNode* pnode) const
{
if (AppliesTo(pnode->nVersion, pnode->strSubVer) ||
AppliesToMe() ||
GetAdjustedTime() < nRelayUntil)
GetTime() < nRelayUntil)
{
pnode->PushMessage("alert", *this);
return true;
Expand Down
23 changes: 23 additions & 0 deletions src/chain.h
Expand Up @@ -18,6 +18,29 @@

static const int SPROUT_VALUE_VERSION = 2001400;

/**
* Maximum amount of time that a block timestamp is allowed to be ahead of the
* current local time.
*/
static const int64_t MAX_FUTURE_BLOCK_TIME_LOCAL = 2 * 60 * 60;

/**
* Maximum amount of time that a block timestamp is allowed to be ahead of the
* median-time-past of the previous block.
*/
static const int64_t MAX_FUTURE_BLOCK_TIME_MTP = 90 * 60;

/**
* Timestamp window used as a grace period by code that compares external
* timestamps (such as timestamps passed to RPCs, or wallet key creation times)
* to block timestamps.
*/
static const int64_t TIMESTAMP_WINDOW = MAX_FUTURE_BLOCK_TIME_LOCAL + 60;

BOOST_STATIC_ASSERT(MAX_FUTURE_BLOCK_TIME_LOCAL > MAX_FUTURE_BLOCK_TIME_MTP);
BOOST_STATIC_ASSERT(TIMESTAMP_WINDOW > MAX_FUTURE_BLOCK_TIME_LOCAL);


struct CDiskBlockPos
{
int nFile;
Expand Down
47 changes: 47 additions & 0 deletions src/gtest/test_forkmanager.cpp
Expand Up @@ -175,3 +175,50 @@ TEST(ForkManager, GetMinimumTimeMainnet) {
EXPECT_EQ(ForkManager::getInstance().getMinimumTime(110001),1496187000);
EXPECT_EQ(ForkManager::getInstance().getMinimumTime(344700),1496187000);
}

TEST(ForkManager, FutureTimeStampMainet) {
SelectParams(CBaseChainParams::MAIN);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(0), false);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(2), false);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(110001), false);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(455555), false);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(740599), false);
int futureTimeStampActivation = 740600;
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(futureTimeStampActivation), true);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(futureTimeStampActivation+144), true);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(futureTimeStampActivation+576), true);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(futureTimeStampActivation+1152), true);
EXPECT_EQ(ForkManager::getInstance().isFutureTimeStampActive(futureTimeStampActivation), false);
EXPECT_EQ(ForkManager::getInstance().isFutureTimeStampActive(futureTimeStampActivation+144), false);
EXPECT_EQ(ForkManager::getInstance().isFutureTimeStampActive(futureTimeStampActivation+576), true);
EXPECT_EQ(ForkManager::getInstance().isFutureTimeStampActive(futureTimeStampActivation+1152), true);
}

TEST(ForkManager, FutureTimeStampTestnet) {
SelectParams(CBaseChainParams::TESTNET);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(0), false);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(2), false);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(70001), false);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(369900), false);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(651099), false);
int futureTimeStampActivation = 651100;
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(futureTimeStampActivation), true);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(futureTimeStampActivation+144), true);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(futureTimeStampActivation+576), true);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(futureTimeStampActivation+1152), true);
EXPECT_EQ(ForkManager::getInstance().isFutureTimeStampActive(futureTimeStampActivation), false);
EXPECT_EQ(ForkManager::getInstance().isFutureTimeStampActive(futureTimeStampActivation+144), false);
EXPECT_EQ(ForkManager::getInstance().isFutureTimeStampActive(futureTimeStampActivation+576), true);
EXPECT_EQ(ForkManager::getInstance().isFutureTimeStampActive(futureTimeStampActivation+1152), true);
}

TEST(ForkManager, FutureTimeStampRegtest) {
SelectParams(CBaseChainParams::REGTEST);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(0), false);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(2), false);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(200), false);
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(249), false);
int futureTimeStampActivation = 250;
EXPECT_EQ(ForkManager::getInstance().isFutureMiningTimeStampActive(futureTimeStampActivation), true);
EXPECT_EQ(ForkManager::getInstance().isFutureTimeStampActive(futureTimeStampActivation), true);
}
100 changes: 100 additions & 0 deletions src/gtest/test_timedata.cpp
@@ -0,0 +1,100 @@
// Copyright (c) 2020 The Zcash developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or https://www.opensource.org/licenses/mit-license.php .

#include <gtest/gtest.h>
#include <gmock/gmock.h>

#include "timedata.h"
#include "random.h"
#include "netbase.h"
#include "utiltime.h"

using ::testing::StrictMock;

class MockCTimeWarning : public CTimeWarning
{
public:
MOCK_METHOD2(Warn, void(size_t, size_t));
};

CNetAddr GetUniqueAddr() {
uint8_t buf[16] = { 0xFD }; // RFC 4193 address in FC00::/7 with L = 1 (locally assigned)
GetRandBytes(&buf[1], 15);
CNetAddr ip;
ip.SetRaw(NET_IPV6, buf);
assert(ip.IsRFC4193());
return ip;
}

TEST(TimeWarning, Assertions)
{
StrictMock<MockCTimeWarning> tw;
EXPECT_DEATH(tw.AddTimeData(GetUniqueAddr(), 0, INT64_MIN), "");
EXPECT_DEATH(tw.AddTimeData(GetUniqueAddr(), 0, -1), "");
EXPECT_DEATH(tw.AddTimeData(GetUniqueAddr(), 0, INT64_MAX - CTimeWarning::TIMEDATA_IGNORE_THRESHOLD + 1), "");
EXPECT_DEATH(tw.AddTimeData(GetUniqueAddr(), 0, INT64_MAX), "");
}

TEST(TimeWarning, NoWarning)
{
StrictMock<MockCTimeWarning> tw;
int64_t now = GetTime();

EXPECT_CALL(tw, Warn(CTimeWarning::TIMEDATA_WARNING_SAMPLES, 0)).Times(0);

tw.AddTimeData(GetUniqueAddr(), now - CTimeWarning::TIMEDATA_IGNORE_THRESHOLD, now);
tw.AddTimeData(GetUniqueAddr(), now + CTimeWarning::TIMEDATA_IGNORE_THRESHOLD, now);
tw.AddTimeData(GetUniqueAddr(), now - CTimeWarning::TIMEDATA_WARNING_THRESHOLD, now);
tw.AddTimeData(GetUniqueAddr(), now + CTimeWarning::TIMEDATA_WARNING_THRESHOLD, now);

for (size_t i = 0; i < CTimeWarning::TIMEDATA_WARNING_SAMPLES - 2; i++) {
tw.AddTimeData(GetUniqueAddr(), now + CTimeWarning::TIMEDATA_WARNING_THRESHOLD + 1, now);
}
CNetAddr duplicateIp = GetUniqueAddr();
for (size_t i = 0; i < 2; i++) {
tw.AddTimeData(duplicateIp, now + CTimeWarning::TIMEDATA_WARNING_THRESHOLD + 1, now);
}
}

TEST(TimeWarning, PeersAhead)
{
StrictMock<MockCTimeWarning> tw;
int64_t now = GetTime();

EXPECT_CALL(tw, Warn(CTimeWarning::TIMEDATA_WARNING_SAMPLES, 0));

for (size_t i = 0; i < CTimeWarning::TIMEDATA_WARNING_SAMPLES - 1; i++) {
tw.AddTimeData(GetUniqueAddr(), now + CTimeWarning::TIMEDATA_WARNING_THRESHOLD + 1, now);
}
tw.AddTimeData(GetUniqueAddr(), now + CTimeWarning::TIMEDATA_IGNORE_THRESHOLD - 1, now);
}

TEST(TimeWarning, PeersBehind)
{
StrictMock<MockCTimeWarning> tw;
int64_t now = GetTime();

EXPECT_CALL(tw, Warn(0, CTimeWarning::TIMEDATA_WARNING_SAMPLES));

for (size_t i = 0; i < CTimeWarning::TIMEDATA_WARNING_SAMPLES - 1; i++) {
tw.AddTimeData(GetUniqueAddr(), now - CTimeWarning::TIMEDATA_WARNING_THRESHOLD - 1, now);
}
tw.AddTimeData(GetUniqueAddr(), now - CTimeWarning::TIMEDATA_IGNORE_THRESHOLD + 1, now);
}

TEST(TimeWarning, PeersMixed)
{
StrictMock<MockCTimeWarning> tw;
int64_t now = GetTime();

EXPECT_CALL(tw, Warn(CTimeWarning::TIMEDATA_WARNING_SAMPLES/2, (CTimeWarning::TIMEDATA_WARNING_SAMPLES+1)/2));

for (size_t i = 0; i < CTimeWarning::TIMEDATA_WARNING_SAMPLES/2; i++) {
tw.AddTimeData(GetUniqueAddr(), now + CTimeWarning::TIMEDATA_WARNING_THRESHOLD + 1, now);
}
for (size_t i = 0; i < (CTimeWarning::TIMEDATA_WARNING_SAMPLES-1)/2; i++) {
tw.AddTimeData(GetUniqueAddr(), now - CTimeWarning::TIMEDATA_WARNING_THRESHOLD - 1, now);
}
tw.AddTimeData(GetUniqueAddr(), now - CTimeWarning::TIMEDATA_IGNORE_THRESHOLD + 1, now);
}
1 change: 1 addition & 0 deletions src/init.cpp
Expand Up @@ -1021,6 +1021,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
fPruneMode = true;
}


#ifdef ENABLE_WALLET
bool fDisableWallet = GetBoolArg("-disablewallet", false);
#endif
Expand Down

0 comments on commit e4d88b2

Please sign in to comment.