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

Discovery ENR extension - server part #5571

Merged
merged 10 commits into from
May 6, 2019
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## [1.7.0] - Unreleased

- Added: [#5537](https://github.com/ethereum/aleth/pull/5537) Creating Ethereum Node Record (ENR) at program start.
- Added: [#5571](https://github.com/ethereum/aleth/pull/5571) Support Discovery v4 ENR Extension messages.
- Added: [#5557](https://github.com/ethereum/aleth/pull/5557) Improved debug logging of full sync.
- Added: [#5564](https://github.com/ethereum/aleth/pull/5564) Improved help output of Aleth by adding list of channels.
- Added: [#5575](https://github.com/ethereum/aleth/pull/5575) Log active peer count and peer list every 30 seconds.
Expand Down
2 changes: 1 addition & 1 deletion aleth/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ int main(int argc, char** argv)
setDefaultOrCLocale();

// Init secp256k1 context by calling one of the functions.
toPublic({});
toPublic(Secret{});

// Init defaults
Ethash::init();
Expand Down
20 changes: 20 additions & 0 deletions libdevcrypto/Common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,26 @@ Public dev::toPublic(Secret const& _secret)
return Public{&serializedPubkey[1], Public::ConstructFromPointer};
}

Public dev::toPublic(PublicCompressed const& _publicCompressed)
{
auto* ctx = getCtx();

secp256k1_pubkey rawPubkey;
if (!secp256k1_ec_pubkey_parse(
Copy link
Member Author

Choose a reason for hiding this comment

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

ctx, &rawPubkey, _publicCompressed.data(), PublicCompressed::size))
return {};

std::array<byte, 65> serializedPubkey;
auto serializedPubkeySize = serializedPubkey.size();
secp256k1_ec_pubkey_serialize(
ctx, serializedPubkey.data(), &serializedPubkeySize, &rawPubkey, SECP256K1_EC_UNCOMPRESSED);
assert(serializedPubkeySize == serializedPubkey.size());
// Expect single byte header of value 0x04 -- uncompressed public key.
assert(serializedPubkey[0] == 0x04);
// Create the Public skipping the header.
return Public{&serializedPubkey[1], Public::ConstructFromPointer};
}

PublicCompressed dev::toPublicCompressed(Secret const& _secret)
{
PublicCompressed serializedPubkey;
Expand Down
3 changes: 3 additions & 0 deletions libdevcrypto/Common.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ using Secrets = std::vector<Secret>;
/// Convert a secret key into the public key equivalent.
Public toPublic(Secret const& _secret);

/// Convert a compressed public key into the uncompressed equivalent.
Public toPublic(PublicCompressed const& _publicCompressed);

/// Convert a secret key into the public key in compressed format.
PublicCompressed toPublicCompressed(Secret const& _secret);

Expand Down
4 changes: 2 additions & 2 deletions libp2p/ENR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ bytes addressToBytes(Address const& _address)
}
} // namespace

ENR::ENR(RLP _rlp, VerifyFunction const& _verifyFunction)
ENR::ENR(RLP const& _rlp, VerifyFunction const& _verifyFunction)
{
if (_rlp.data().size() > c_ENRMaxSizeBytes)
BOOST_THROW_EXCEPTION(ENRIsTooBig());
Expand Down Expand Up @@ -110,7 +110,7 @@ ENR createV4ENR(Secret const& _secret, boost::asio::ip::address const& _ip, uint
return ENR{0 /* sequence number */, keyValuePairs, signFunction};
}

ENR parseV4ENR(RLP _rlp)
ENR parseV4ENR(RLP const& _rlp)
{
ENR::VerifyFunction verifyFunction = [](std::map<std::string, bytes> const& _keyValuePairs,
bytesConstRef _signature, bytesConstRef _data) {
Expand Down
4 changes: 2 additions & 2 deletions libp2p/ENR.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class ENR
std::function<bool(std::map<std::string, bytes> const&, bytesConstRef, bytesConstRef)>;

// Parse from RLP with given signature verification function
ENR(RLP _rlp, VerifyFunction const& _verifyFunction);
ENR(RLP const& _rlp, VerifyFunction const& _verifyFunction);
// Create with given sign function
ENR(uint64_t _seq, std::map<std::string, bytes> const& _keyValues,
SignFunction const& _signFunction);
Expand All @@ -54,7 +54,7 @@ class ENR

ENR createV4ENR(Secret const& _secret, boost::asio::ip::address const& _ip, uint16_t _tcpPort, uint16_t _udpPort);

ENR parseV4ENR(RLP _rlp);
ENR parseV4ENR(RLP const& _rlp);

std::ostream& operator<<(std::ostream& _out, ENR const& _enr);

Expand Down
8 changes: 2 additions & 6 deletions libp2p/Host.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -800,13 +800,9 @@ void Host::startedWorking()
m_listenPort = m_netConfig.listenPort;

determinePublic();
auto nodeTable = make_shared<NodeTable>(
m_ioService,
m_alias,
auto nodeTable = make_shared<NodeTable>(m_ioService, m_alias,
NodeIPEndpoint(bi::address::from_string(listenAddress()), listenPort(), listenPort()),
m_netConfig.discovery,
m_netConfig.allowLocalDiscovery
);
m_enr, m_netConfig.discovery, m_netConfig.allowLocalDiscovery);

// Don't set an event handler if we don't have capabilities, because no capabilities
// means there's no host state to update in response to node table events
Expand Down
90 changes: 85 additions & 5 deletions libp2p/NodeTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ inline bool operator==(weak_ptr<NodeEntry> const& _weak, shared_ptr<NodeEntry> c
}

NodeTable::NodeTable(ba::io_service& _io, KeyPair const& _alias, NodeIPEndpoint const& _endpoint,
bool _enabled, bool _allowLocalDiscovery)
ENR const& _enr, bool _enabled, bool _allowLocalDiscovery)
: m_hostNodeID{_alias.pub()},
m_hostNodeEndpoint{_endpoint},
m_hostENR{_enr},
m_secret{_alias.secret()},
m_socket{make_shared<NodeSocket>(
_io, static_cast<UDPSocketEvents&>(*this), (bi::udp::endpoint)m_hostNodeEndpoint)},
Expand Down Expand Up @@ -220,7 +221,7 @@ void NodeTable::doDiscoveryRound(
}

FindNode p(nodeEntry->endpoint(), _node);
p.ts = nextRequestExpirationTime();
p.expiration = nextRequestExpirationTime();
p.sign(m_secret);
m_sentFindNodes.emplace_back(nodeEntry->id(), chrono::steady_clock::now());
LOG(m_logger) << p.typeName() << " to " << nodeEntry->node << " (target: " << _node
Expand Down Expand Up @@ -302,7 +303,8 @@ void NodeTable::ping(Node const& _node, shared_ptr<NodeEntry> _replacementNodeEn
}

PingNode p{m_hostNodeEndpoint, _node.endpoint};
p.ts = nextRequestExpirationTime();
p.expiration = nextRequestExpirationTime();
p.seq = m_hostENR.sequenceNumber();
auto const pingHash = p.sign(m_secret);
LOG(m_logger) << p.typeName() << " to " << _node;
m_socket->send(p);
Expand Down Expand Up @@ -454,6 +456,14 @@ void NodeTable::onPacketReceived(
case PingNode::type:
sourceNodeEntry = handlePingNode(_from, *packet);
break;

case ENRRequest::type:
sourceNodeEntry = handleENRRequest(_from, *packet);
break;

case ENRResponse::type:
sourceNodeEntry = handleENRResponse(_from, *packet);
break;
}

if (sourceNodeEntry)
Expand Down Expand Up @@ -608,7 +618,7 @@ std::shared_ptr<NodeEntry> NodeTable::handleFindNode(
for (unsigned offset = 0; offset < nearest.size(); offset += nlimit)
{
Neighbours out(_from, nearest, offset, nlimit);
out.ts = nextRequestExpirationTime();
out.expiration = nextRequestExpirationTime();
LOG(m_logger) << out.typeName() << " to " << in.sourceid << "@" << _from;
out.sign(m_secret);
if (out.data.size() > 1280)
Expand All @@ -631,8 +641,9 @@ std::shared_ptr<NodeEntry> NodeTable::handlePingNode(
// Send PONG response.
Pong p(sourceEndpoint);
LOG(m_logger) << p.typeName() << " to " << in.sourceid << "@" << _from;
p.ts = nextRequestExpirationTime();
p.expiration = nextRequestExpirationTime();
p.echo = in.echo;
p.seq = m_hostENR.sequenceNumber();
p.sign(m_secret);
m_socket->send(p);

Expand All @@ -647,6 +658,69 @@ std::shared_ptr<NodeEntry> NodeTable::handlePingNode(
return sourceNodeEntry;
}

std::shared_ptr<NodeEntry> NodeTable::handleENRRequest(
bi::udp::endpoint const& _from, DiscoveryDatagram const& _packet)
{
std::shared_ptr<NodeEntry> sourceNodeEntry = nodeEntry(_packet.sourceid);
if (!sourceNodeEntry)
{
LOG(m_logger) << "Source node (" << _packet.sourceid << "@" << _from
<< ") not found in node table. Ignoring ENRRequest request.";
return {};
}
if (sourceNodeEntry->endpoint() != _from)
{
LOG(m_logger) << "ENRRequest packet from unexpected endpoint " << _from << " instead of "
halfalicious marked this conversation as resolved.
Show resolved Hide resolved
<< sourceNodeEntry->endpoint();
halfalicious marked this conversation as resolved.
Show resolved Hide resolved
return {};
}
if (!sourceNodeEntry->lastPongReceivedTime)
{
LOG(m_logger) << "Unexpected ENRRequest packet! Endpoint proof hasn't been performed yet.";
return {};
}
if (!sourceNodeEntry->hasValidEndpointProof())
{
LOG(m_logger) << "Unexpected ENRRequest packet! Endpoint proof has expired.";
return {};
}

auto const& in = dynamic_cast<ENRRequest const&>(_packet);

ENRResponse response{_from, m_hostENR};
LOG(m_logger) << response.typeName() << " to " << in.sourceid << "@" << _from;
response.expiration = nextRequestExpirationTime();
response.echo = in.echo;
response.sign(m_secret);
m_socket->send(response);

return sourceNodeEntry;
}

std::shared_ptr<NodeEntry> NodeTable::handleENRResponse(
Copy link
Contributor

Choose a reason for hiding this comment

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

We should verify the hash in the response against the request (or are you planning on doing that in another change)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I planned to add the validation when implementing doing something useful with these responses (the last "ENR Client" part in #5551)
But now I also think there's not much useful we can actually do with them in v4 discovery, so maybe I'll leave it just ignoring ENRResponses

Copy link
Contributor

@halfalicious halfalicious May 3, 2019

Choose a reason for hiding this comment

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

If we listen for responses I think we should validate them so there's 1 less thing to do when we want to actually do something useful with them.

bi::udp::endpoint const& _from, DiscoveryDatagram const& _packet)
{
std::shared_ptr<NodeEntry> sourceNodeEntry = nodeEntry(_packet.sourceid);
if (!sourceNodeEntry)
{
LOG(m_logger) << "Source node (" << _packet.sourceid << "@" << _from
<< ") not found in node table. Ignoring ENRResponse packet.";
return {};
}
if (sourceNodeEntry->endpoint() != _from)
{
LOG(m_logger) << "ENRResponse packet from unexpected endpoint " << _from << " instead of "
gumb0 marked this conversation as resolved.
Show resolved Hide resolved
<< sourceNodeEntry->endpoint();
return {};
}

auto const& in = dynamic_cast<ENRResponse const&>(_packet);
LOG(m_logger) << "Received ENR: " << *in.enr;

return sourceNodeEntry;
}


void NodeTable::doDiscovery()
{
m_discoveryTimer->expires_from_now(c_bucketRefreshMs);
Expand Down Expand Up @@ -766,6 +840,12 @@ unique_ptr<DiscoveryDatagram> DiscoveryDatagram::interpretUDP(bi::udp::endpoint
case Neighbours::type:
decoded.reset(new Neighbours(_from, sourceid, echo));
break;
case ENRRequest::type:
decoded.reset(new ENRRequest(_from, sourceid, echo));
break;
case ENRResponse::type:
decoded.reset(new ENRResponse(_from, sourceid, echo));
break;
default:
LOG(g_discoveryWarnLogger::get()) << "Invalid packet (unknown packet type) from "
<< _from.address().to_string() << ":" << _from.port();
Expand Down
Loading