Remove jsonrpc dependency from PortalnetProtocol struct and change default folder name #80
Conversation
ogenev
force-pushed
the
refactor-portal-protocol
branch
from
5d4df7b to
562e763
Compare
ogenev
force-pushed
the
refactor-portal-protocol
branch
from
562e763 to
6333f07
Compare
ogenev
changed the title
| let proto = Self { | ||
| discovery: discovery.clone(), | ||
| overlay: overlay.clone(), |
There was a problem hiding this comment.
Was there anything important about the move of let proto? It's helpful to me as a reviewer when a PR avoids moving code if the move has no effect, to make the diff smaller and quicker to review.
There was a problem hiding this comment.
That's right, I will refrain from such moves in the future.
| // Append last 8 enr base64 encoded chars to application dir name | ||
| let mut application_string = "Trin_".to_owned(); | ||
| let len = &local_enr.to_base64().len(); | ||
| let suffix = &local_enr.to_base64()[len - 8..]; |
There was a problem hiding this comment.
If I were new to trin, and found a directory named like this, I would expect it to be the first few characters of the as-displayed node-id. I understand if that feels like a tangent to you right now, and if so you want to skip it. In that case, maybe you could add a "good first issue" with that suggestion? Seems like a nice bite-size chunk for a newbie who wants to get started.
Sometimes, we may want to initialize a portal net protocol without jsonrpc support (for example when running tests or creating "dummy" node).
It will be also good if we can run multiple portal protocol instances simultaneously (for testing req/resp network calls). The current default db folder structure prevent this, because there is a db collusion between the different instances.
This PR removes jsonrpc dependency from PortalnetProtocol struct and adds last 8 chars from local base64 encoded Enr as a suffix to the default data application folder.