Update NodeRecord on DiscoveryPeer (#2047)

We wrote the saved the new node record to disk but we didn't set it on the DiscoveryPeer class so we were still responding with the old one.

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>
pull/2057/head
Ratan (Rai) Sur 4 years ago committed by GitHub
parent a58c472480
commit e88ac27f97
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 86
      ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java
  2. 25
      ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java

@ -166,7 +166,7 @@ public abstract class PeerDiscoveryAgent {
this.localNode = Optional.of(ourNode);
isActive = true;
LOG.info("P2P peer discovery agent started and listening on {}", localAddress);
ourNode.setNodeRecord(updateNodeRecord());
updateNodeRecord();
startController(ourNode);
return discoveryPort;
});
@ -191,43 +191,48 @@ public abstract class PeerDiscoveryAgent {
final Integer discoveryPort = maybeEnodeURL.flatMap(EnodeURL::getDiscoveryPort).orElse(0);
final Integer listeningPort = maybeEnodeURL.flatMap(EnodeURL::getListeningPort).orElse(0);
final String forkIdEnrField = "eth";
return existingNodeRecord
.filter(
nodeRecord ->
id.equals(nodeRecord.get(EnrField.PKEY_SECP256K1))
&& addressBytes.equals(nodeRecord.get(EnrField.IP_V4))
&& discoveryPort.equals(nodeRecord.get(EnrField.UDP))
&& listeningPort.equals(nodeRecord.get(EnrField.TCP))
&& forkIdSupplier.get().equals(nodeRecord.get(forkIdEnrField)))
.orElseGet(
() -> {
final UInt64 sequenceNumber =
existingNodeRecord.map(NodeRecord::getSeq).orElse(UInt64.ZERO).add(1);
final NodeRecord nodeRecord =
nodeRecordFactory.createFromValues(
sequenceNumber,
new EnrField(EnrField.ID, IdentitySchema.V4),
new EnrField(EnrField.PKEY_SECP256K1, Functions.compressPublicKey(id)),
new EnrField(EnrField.IP_V4, addressBytes),
new EnrField(EnrField.TCP, listeningPort),
new EnrField(EnrField.UDP, discoveryPort),
new EnrField(
forkIdEnrField, Collections.singletonList(forkIdSupplier.get())));
nodeRecord.setSignature(
nodeKey
.sign(Hash.keccak256(nodeRecord.serializeNoSignature()))
.encodedBytes()
.slice(0, 64));
LOG.info("Writing node record to disk. {}", nodeRecord);
final KeyValueStorageTransaction keyValueStorageTransaction =
keyValueStorage.startTransaction();
keyValueStorageTransaction.put(
Bytes.wrap(SEQ_NO_STORE_KEY.getBytes(UTF_8)).toArray(),
nodeRecord.serialize().toArray());
keyValueStorageTransaction.commit();
return nodeRecord;
});
final NodeRecord newNodeRecord =
existingNodeRecord
.filter(
nodeRecord ->
id.equals(nodeRecord.get(EnrField.PKEY_SECP256K1))
&& addressBytes.equals(nodeRecord.get(EnrField.IP_V4))
&& discoveryPort.equals(nodeRecord.get(EnrField.UDP))
&& listeningPort.equals(nodeRecord.get(EnrField.TCP))
&& forkIdSupplier.get().equals(nodeRecord.get(forkIdEnrField)))
.orElseGet(
() -> {
final UInt64 sequenceNumber =
existingNodeRecord.map(NodeRecord::getSeq).orElse(UInt64.ZERO).add(1);
final NodeRecord nodeRecord =
nodeRecordFactory.createFromValues(
sequenceNumber,
new EnrField(EnrField.ID, IdentitySchema.V4),
new EnrField(EnrField.PKEY_SECP256K1, Functions.compressPublicKey(id)),
new EnrField(EnrField.IP_V4, addressBytes),
new EnrField(EnrField.TCP, listeningPort),
new EnrField(EnrField.UDP, discoveryPort),
new EnrField(
forkIdEnrField, Collections.singletonList(forkIdSupplier.get())));
nodeRecord.setSignature(
nodeKey
.sign(Hash.keccak256(nodeRecord.serializeNoSignature()))
.encodedBytes()
.slice(0, 64));
LOG.info("Writing node record to disk. {}", nodeRecord);
final KeyValueStorageTransaction keyValueStorageTransaction =
keyValueStorage.startTransaction();
keyValueStorageTransaction.put(
Bytes.wrap(SEQ_NO_STORE_KEY.getBytes(UTF_8)).toArray(),
nodeRecord.serialize().toArray());
keyValueStorageTransaction.commit();
return nodeRecord;
});
localNode
.orElseThrow(() -> new IllegalStateException("Local node should be set here"))
.setNodeRecord(newNodeRecord);
return newNodeRecord;
}
public void addPeerRequirement(final PeerRequirement peerRequirement) {
@ -392,4 +397,9 @@ public abstract class PeerDiscoveryAgent {
DiscoveryPeer.from(peer).ifPresent(c::handleBondingRequest);
});
}
@VisibleForTesting
public Optional<DiscoveryPeer> getLocalNode() {
return localNode;
}
}

@ -112,6 +112,31 @@ public class PeerDiscoveryAgentTest {
+ "v0AHacwUAPMljNMTiDdGNwAoN1ZHCCdl8");
}
@Test
public void testNodeRecordCreatedUpdatesDiscoveryPeer() {
KeyPair keyPair =
SIGNATURE_ALGORITHM
.get()
.createKeyPair(
SIGNATURE_ALGORITHM
.get()
.createPrivateKey(
Bytes32.fromHexString(
"0xb71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")));
final MockPeerDiscoveryAgent agent =
helper.startDiscoveryAgent(
helper
.agentBuilder()
.nodeKey(NodeKeyUtils.createFrom(keyPair))
.advertisedHost("127.0.0.1")
.bindPort(30303));
agent.start(30303);
final NodeRecord pre = agent.getLocalNode().get().getNodeRecord().get();
agent.updateNodeRecord();
final NodeRecord post = agent.getLocalNode().get().getNodeRecord().get();
assertThat(pre).isNotEqualTo(post);
}
@Test
public void neighborsPacketFromUnbondedPeerIsDropped() {
// Start an agent with no bootstrap peers.

Loading…
Cancel
Save