Skip to content
This repository has been archived by the owner on Oct 28, 2021. It is now read-only.

Fix logic used to determine if Aleth should request dao fork block header from potential peer #5539

Merged
merged 5 commits into from
Apr 2, 2019
Merged

Fix logic used to determine if Aleth should request dao fork block header from potential peer #5539

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b037fbe
Add check for infinite block number to dao check
halfalicious Apr 1, 2019
841c7d8
Update license messages and add constexpr
halfalicious Apr 1, 2019
37b2dc2
Fix misspelling in constant c_infiniteBlockNumer
halfalicious Apr 1, 2019
d70051b
Remove unnecessary static + switch constexpr/type
halfalicious Apr 2, 2019
a32de03
Update changelog
halfalicious Apr 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions libethcore/ChainOperationParams.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class PrecompiledContract
u256 m_startingBlock = 0;
};

static constexpr int64_t c_infiniteBlockNumer = std::numeric_limits<int64_t>::max();
static constexpr int64_t c_infiniteBlockNumber = std::numeric_limits<int64_t>::max();
Copy link
Member

Choose a reason for hiding this comment

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

The static is redundant for constexpr.

Copy link
Contributor Author

@halfalicious halfalicious Apr 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I thought that static and constexpr are orthogonal concepts? I agree that static doesn't make sense here (regardless of the constexpr) since this is a header file and c_infiniteBlockNumber is defined outside of a class - from what I understand, static when used this way limits the scope of the variable to the translation unit which means that each cpp file which includes the header will have a separate instance of c_infiniteBlockNumber (though each instance will have the same value) which seems unnecessary.

Copy link
Member

Choose a reason for hiding this comment

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

I'm also confused now. On the language level that might be true. But also constexpr implies inline and inline implies static.

I check than in practice the constexpr symbols are not exported.


struct ChainOperationParams
{
Expand All @@ -87,15 +87,15 @@ struct ChainOperationParams
u256 minGasLimit;
u256 maxGasLimit;
u256 gasLimitBoundDivisor;
u256 homesteadForkBlock = c_infiniteBlockNumer;
u256 EIP150ForkBlock = c_infiniteBlockNumer;
u256 EIP158ForkBlock = c_infiniteBlockNumer;
u256 byzantiumForkBlock = c_infiniteBlockNumer;
u256 eWASMForkBlock = c_infiniteBlockNumer;
u256 constantinopleForkBlock = c_infiniteBlockNumer;
u256 constantinopleFixForkBlock = c_infiniteBlockNumer;
u256 daoHardforkBlock = c_infiniteBlockNumer;
u256 experimentalForkBlock = c_infiniteBlockNumer;
u256 homesteadForkBlock = c_infiniteBlockNumber;
u256 EIP150ForkBlock = c_infiniteBlockNumber;
u256 EIP158ForkBlock = c_infiniteBlockNumber;
u256 byzantiumForkBlock = c_infiniteBlockNumber;
u256 eWASMForkBlock = c_infiniteBlockNumber;
u256 constantinopleForkBlock = c_infiniteBlockNumber;
u256 constantinopleFixForkBlock = c_infiniteBlockNumber;
u256 daoHardforkBlock = c_infiniteBlockNumber;
u256 experimentalForkBlock = c_infiniteBlockNumber;
int chainID = 0; // Distinguishes different chains (mainnet, Ropsten, etc).
int networkID = 0; // Distinguishes different sub protocols.

Expand Down
36 changes: 9 additions & 27 deletions libethereum/BlockChainSync.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,6 @@
/*
This file is part of cpp-ethereum.

cpp-ethereum is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

cpp-ethereum is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with cpp-ethereum. If not, see <http://www.gnu.org/licenses/>.
*/
/** @file BlockChainSync.cpp
* @author Gav Wood <i@gavwood.com>
* @date 2014
*/
/// Aleth: Ethereum C++ client, tools and libraries.
// Copyright 2019 Aleth Authors.
// Licensed under the GNU General Public License, Version 3.

#include "BlockChainSync.h"

Expand All @@ -35,11 +18,6 @@ using namespace std;
using namespace dev;
using namespace dev::eth;

unsigned const c_maxPeerUknownNewBlocks = 1024; /// Max number of unknown new blocks peer can give us
unsigned const c_maxRequestHeaders = 1024;
unsigned const c_maxRequestBodies = 1024;


std::ostream& dev::eth::operator<<(std::ostream& _out, SyncStatus const& _sync)
{
_out << "protocol: " << _sync.protocolVersion << endl;
Expand All @@ -52,6 +30,10 @@ std::ostream& dev::eth::operator<<(std::ostream& _out, SyncStatus const& _sync)
namespace // Helper functions.
{

unsigned constexpr c_maxPeerUknownNewBlocks = 1024; /// Max number of unknown new blocks peer can give us
Copy link
Member

Choose a reason for hiding this comment

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

Pedantic mode: should be constexpr unsigned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why do we put constexpr before the type but const after the type?

unsigned constexpr c_maxRequestHeaders = 1024;
unsigned constexpr c_maxRequestBodies = 1024;

template<typename T> bool haveItem(std::map<unsigned, T>& _container, unsigned _number)
{
if (_container.empty())
Expand Down Expand Up @@ -230,8 +212,8 @@ void BlockChainSync::onPeerStatus(EthereumPeer const& _peer)
bool BlockChainSync::requestDaoForkBlockHeader(NodeID const& _peerID)
{
// DAO challenge
unsigned const daoHardfork = static_cast<unsigned>(host().chain().sealEngine()->chainParams().daoHardforkBlock);
if (daoHardfork == 0)
u256 const daoHardfork = host().chain().sealEngine()->chainParams().daoHardforkBlock;
if (daoHardfork == 0 || daoHardfork == c_infiniteBlockNumber)
return false;

m_daoChallengedPeers.insert(_peerID);
Expand Down
23 changes: 3 additions & 20 deletions libethereum/BlockChainSync.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,6 @@
/*
This file is part of cpp-ethereum.

cpp-ethereum is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

cpp-ethereum is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with cpp-ethereum. If not, see <http://www.gnu.org/licenses/>.
*/
/** @file BlockChainSync.h
* @author Gav Wood <i@gavwood.com>
* @date 2014
*/
// Aleth: Ethereum C++ client, tools and libraries.
// Copyright 2019 Aleth Authors.
// Licensed under the GNU General Public License, Version 3.

#pragma once

Expand Down