Output ENR text representation in admin.nodeInfo RPC#5697
Output ENR text representation in admin.nodeInfo RPC#5697twinstar26 wants to merge 1 commit intoethereum:masterfrom twinstar26:aleth-depfunc
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5697 +/- ##
==========================================
- Coverage 62.97% 62.76% -0.21%
==========================================
Files 350 349 -1
Lines 29994 29744 -250
Branches 3362 3349 -13
==========================================
- Hits 18888 18670 -218
+ Misses 9886 9862 -24
+ Partials 1220 1212 -8 |
gumb0
left a comment
There was a problem hiding this comment.
Instead of processing the result of toBase64 it's better to create another similar function, then make both functions implemented in terms of the third internal function, taking bytesConstRef _in and the chars that vary as parameters.
It could be for example string toBase64(bytesConstRef _in, char const* _base64_chars, bool _pad)
(then define two variants of base64_chars array to pass there; _pad flags enables padding with =)
gumb0
left a comment
There was a problem hiding this comment.
Add a unit test for encoding at least.
(at most for RPC method, too)
| bytes content() const; | ||
| size_t contentRlpListItemCount() const { return m_keyValuePairs.size() * 2 + 1; } | ||
| void streamContent(RLPStream& _s) const; | ||
| Host* h; |
There was a problem hiding this comment.
This is not correct (ENR can belong to some peer, not to Host) and not needed, just use streamRLP of this object in encodeEnr
| { | ||
|
|
||
| std::string toBase64(bytesConstRef _in); | ||
| std::string toBase64url(bytesConstRef _in); |
|
|
||
| #pragma once | ||
|
|
||
| #include <libdevcore/Base64.h> |
| ENR update( | ||
| std::map<std::string, bytes> const& _keyValuePair, SignFunction const& _signFunction) const; | ||
|
|
||
| std::string encodeEnr(); |
| /// Create with given sign function | ||
| ENR(uint64_t _seq, std::map<std::string, bytes> const& _keyValuePairs, | ||
| SignFunction const& _signFunction); | ||
| ENR() = default; |
| /// Get the node information. | ||
| p2p::NodeInfo nodeInfo() const { return NodeInfo(id(), (networkConfig().publicIPAddress.empty() ? m_tcpPublic.address().to_string() : networkConfig().publicIPAddress), m_tcpPublic.port(), m_clientVersion); } | ||
| p2p::NodeInfo nodeInfo() const { | ||
| ENR e; |
There was a problem hiding this comment.
You will return empty ENR this way, not useful to anyone.
Get it from enr() method
There was a problem hiding this comment.
Also use clang-format for you changes as described in https://github.com/ethereum/aleth/blob/master/CONTRIBUTING.md
There was a problem hiding this comment.
Thanks for the reviews!!
Oh some sort of wrapper functionality?? |
|
There will be one internal function, and then two public function will wrap internal one, passing it different arguments. |
|
Replaced by #5701 |
Fixes #5622