Reimplement EthereumNodeRecord for DNS discovery #7989
Conversation
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
| class EthereumNodeRecordTest { | ||
|
|
||
| @Test | ||
| void buildFromRLP() throws Exception { |
There was a problem hiding this comment.
worth adding some tests for error handling? udp / tcp port values?
There was a problem hiding this comment.
updated unit test and added additional test to cover error conditions.
...2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/dns/EthereumNodeRecordTest.java
Outdated
Show resolved
Hide resolved
...2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/dns/EthereumNodeRecordTest.java
Outdated
Show resolved
Hide resolved
| while (!input.isEndOfCurrentList()) { | ||
| var key = new String(input.readBytes().toArrayUnsafe(), StandardCharsets.UTF_8); | ||
| if (input.nextIsList()) { | ||
| // skip list as we currently don't need and support complex structures |
There was a problem hiding this comment.
| // skip list as we currently don't need and support complex structures | |
| // skip list as we currently don't need to support complex structures |
There was a problem hiding this comment.
modified this slightly.
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
|
This pr is stale because it has been open for 30 days with no activity. |
Signed-off-by: Usman Saleem <[email protected]>
| var curve = SignatureAlgorithmFactory.getInstance().getCurve(); | ||
| var ecPoint = curve.getCurve().decodePoint(keyBytes.toArrayUnsafe()); | ||
| return Bytes.wrap(ecPoint.getEncoded(false)).slice(1); |
There was a problem hiding this comment.
Is this equivalent to what the tuweni implementation does?
There was a problem hiding this comment.
correct, the first byte is the encoding prefix i.e. 0x03 for compressed and 0x04 for decompressed public key I recall. We need the uncompressed public key without this prefix.
Signed-off-by: Usman Saleem <[email protected]>
| public boolean equals(final Object o) { | ||
| if (this == o) return true; | ||
| if (o == null || getClass() != o.getClass()) return false; | ||
| EthereumNodeRecord that = (EthereumNodeRecord) o; |
There was a problem hiding this comment.
nit:
if (!(o instanceof EthereumNodeRecord that)) return false;
There was a problem hiding this comment.
I would also encourage putting brackets to follow same code style as other code
There was a problem hiding this comment.
thanks, regenerated equals/hashcode 👍🏼
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
PR description
Reimplement EthereumNodeRecord to avoid dependency on tuweni-devp2p
Fixed Issue(s)
Fixes #7980
Thanks for sending a pull request! Have you done the following?
doc-change-requiredlabel to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew build./gradlew acceptanceTest./gradlew integrationTest./gradlew ethereum:referenceTests:referenceTests