diff --git a/eth/p2p/discoveryv5/enr.nim b/eth/p2p/discoveryv5/enr.nim index eb7c2ca..95d50c2 100644 --- a/eth/p2p/discoveryv5/enr.nim +++ b/eth/p2p/discoveryv5/enr.nim @@ -233,8 +233,8 @@ proc find(r: Record, key: string): Option[int] = if k == key: return some(i) -proc update*(record: Record, pk: PrivateKey, fieldPairs: openarray[FieldPair]): - EnrResult[Record] = +proc update*(record: var Record, pk: PrivateKey, + fieldPairs: openarray[FieldPair]): EnrResult[void] = var r = record let pubkey = r.get(PublicKey) @@ -262,14 +262,15 @@ proc update*(record: Record, pk: PrivateKey, fieldPairs: openarray[FieldPair]): return err("Maximum sequence number reached") r.seqNum.inc() r.raw = ? makeEnrRaw(r.seqNum, pk, r.pairs) + record = r - ok(r) + ok() -proc update*(r: Record, pk: PrivateKey, - ip: Option[ValidIpAddress], - tcpPort, udpPort: Port, - extraFields: openarray[FieldPair] = []): - EnrResult[Record] = +proc update*(r: var Record, pk: PrivateKey, + ip: Option[ValidIpAddress], + tcpPort, udpPort: Port, + extraFields: openarray[FieldPair] = []): + EnrResult[void] = var fields = newSeq[FieldPair]() fields.addAddress(ip, tcpPort, udpPort) diff --git a/eth/p2p/discoveryv5/protocol.nim b/eth/p2p/discoveryv5/protocol.nim index 9396247..8c78f9c 100644 --- a/eth/p2p/discoveryv5/protocol.nim +++ b/eth/p2p/discoveryv5/protocol.nim @@ -178,8 +178,9 @@ func getRecord*(d: Protocol): Record = proc updateRecord*( d: Protocol, enrFields: openarray[(string, seq[byte])]): DiscResult[void] = let fields = mapIt(enrFields, toFieldPair(it[0], it[1])) - d.localNode.record = ? d.localNode.record.update(d.privateKey, fields) - ok() + d.localNode.record.update(d.privateKey, fields) + # TODO: Would it make sense to actively ping ("broadcast") to all the peers + # we stored a handshake with in order to get that ENR updated? proc send(d: Protocol, a: Address, data: seq[byte]) = let ta = initTAddress(a.ip, a.port) @@ -710,18 +711,19 @@ proc newProtocol*(privKey: PrivateKey, db: Database, # Anyhow, nim-beacon-chain would also require some changes to support port # remapping through NAT and this API is also subject to change once we # introduce support for ipv4 + ipv6 binding/listening. - let - extraFields = mapIt(localEnrFields, toFieldPair(it[0], it[1])) - # TODO: - # - Defect as is now or return a result for enr errors? - # - In case incorrect key, allow for new enr based on new key (new node id)? - enr = if previousRecord.isSome(): - previousRecord.get().update(privKey, externalIp, tcpPort, udpPort, - extraFields).expect("Record within size limits and correct key") - else: - enr.Record.init(1, privKey, externalIp, tcpPort, udpPort, - extraFields).expect("Record within size limits") - node = newNode(enr).expect("Properly initialized record") + let extraFields = mapIt(localEnrFields, toFieldPair(it[0], it[1])) + # TODO: + # - Defect as is now or return a result for enr errors? + # - In case incorrect key, allow for new enr based on new key (new node id)? + var record: Record + if previousRecord.isSome(): + record = previousRecord.get() + record.update(privKey, externalIp, tcpPort, udpPort, + extraFields).expect("Record within size limits and correct key") + else: + record = enr.Record.init(1, privKey, externalIp, tcpPort, udpPort, + extraFields).expect("Record within size limits") + let node = newNode(record).expect("Properly initialized record") # TODO Consider whether this should be a Defect doAssert rng != nil, "RNG initialization failed" diff --git a/tests/p2p/test_enr.nim b/tests/p2p/test_enr.nim index c0936db..ac1927d 100644 --- a/tests/p2p/test_enr.nim +++ b/tests/p2p/test_enr.nim @@ -95,26 +95,23 @@ suite "ENR": var r = Record.init(1, pk, none(ValidIpAddress), Port(9000), Port(9000))[] block: # Insert new k:v pair, update of seqNum should occur. - let res = r.update(pk, [newField]) - check res.isOk() - r = res[] + let updated = r.update(pk, [newField]) + check updated.isOk() check: r.get("test", uint) == 123 r.seqNum == 2 block: # Insert same k:v pair, no update of seqNum should occur. - let res = r.update(pk, [newField]) - check res.isOk() - r = res[] + let updated = r.update(pk, [newField]) + check updated.isOk() check: r.get("test", uint) == 123 r.seqNum == 2 block: # Insert k:v pair with changed value, update of seqNum should occur. let updatedField = toFieldPair("test", 1234'u) - let res = r.update(pk, [updatedField]) - check res.isOk() - r = res[] + let updated = r.update(pk, [updatedField]) + check updated.isOk() check: r.get("test", uint) == 1234 r.seqNum == 3 @@ -130,9 +127,9 @@ suite "ENR": let newField = toFieldPair("test", 123'u) let newField2 = toFieldPair("zzz", 123'u) - let res = r.update(pk, [newField, newField2]) - check res.isOk() - check $res[] == """(123: "abc", a12: 1, abc: 1234, id: "v4", secp256k1: 0x02E51EFA66628CE09F689BC2B82F165A75A9DDECBB6A804BE15AC3FDF41F3B34E7, test: 123, z: 0x00, zzz: 123)""" + let updated = r.update(pk, [newField, newField2]) + check updated.isOk() + check $r == """(123: "abc", a12: 1, abc: 1234, id: "v4", secp256k1: 0x02E51EFA66628CE09F689BC2B82F165A75A9DDECBB6A804BE15AC3FDF41F3B34E7, test: 123, z: 0x00, zzz: 123)""" test "ENR update size too big": let pk = PrivateKey.fromHex( @@ -142,8 +139,8 @@ suite "ENR": check r.isOk() let newField = toFieldPair("test", 123'u) - let res = r[].update(pk, [newField]) - check res.isErr() + let updated = r[].update(pk, [newField]) + check updated.isErr() test "ENR update invalid key": let pk = PrivateKey.fromHex( @@ -155,8 +152,8 @@ suite "ENR": let wrongPk = PrivateKey.random(rng[]) newField = toFieldPair("test", 123'u) - res = r[].update(wrongPk, [newField]) - check res.isErr() + updated = r[].update(wrongPk, [newField]) + check updated.isErr() test "ENR update address": let @@ -165,18 +162,16 @@ suite "ENR": var r = Record.init(1, pk, none(ValidIpAddress), Port(9000), Port(9000))[] block: - let res = r.update(pk, none(ValidIpAddress), Port(9000), Port(9000)) - check res.isOk() - r = res[] + let updated = r.update(pk, none(ValidIpAddress), Port(9000), Port(9000)) + check updated.isOk() check: r.get("tcp", uint) == 9000 r.get("udp", uint) == 9000 r.seqNum == 1 block: - let res = r.update(pk, none(ValidIpAddress), Port(9001), Port(9002)) - check res.isOk() - r = res[] + let updated = r.update(pk, none(ValidIpAddress), Port(9001), Port(9002)) + check updated.isOk() check: r.get("tcp", uint) == 9001 r.get("udp", uint) == 9002