feat: add ability to parse addr infos from strings #143
Conversation
Codecov Report
@@ Coverage Diff @@
## main #143 +/- ##
==========================================
+ Coverage 45.90% 46.46% +0.56%
==========================================
Files 15 16 +1
Lines 952 962 +10
==========================================
+ Hits 437 447 +10
Misses 466 466
Partials 49 49
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| logger.Error("parsing info from multiaddr", "addr", addr, "err", err) | ||
| return nil, err | ||
| } | ||
| infos = append(infos, *info) |
There was a problem hiding this comment.
[non blocking question]
dereferencing this value makes sense to me, as I never like using pointers when we don't strictly need to (like for a read only struct such as AddrInfo), especially since its already a slice. However, I'm uncertain if there is a reason why we need a pointer, since the maintainers return a pointer. Do we know why they use a pointer?
There was a problem hiding this comment.
I took the implementation from Celestia-node 🐱
I guess the reason for using pointers is that the addresses could get heavy and we don't want to copy them every time.
What dl you think? Should i switch to using pointers?
There was a problem hiding this comment.
I took the implementation from Celestia-node 🐱
I guess the reason for using pointers is that the addresses could get heavy and we don't want to copy them every time.
What dl you think? Should i switch to using pointers?
There was a problem hiding this comment.
nah, if celestia-node uses it and they don't have issues its probably fine. It should be an improvement to not use pointers, as it makes no sense to me, but I'm just paranoid lol.
There was a problem hiding this comment.
Shit, after merging I remembered that I did couple changes, here is the original implementation:
https://github.com/celestiaorg/celestia-node/blob/bd96b3dab06e7d733cf0e62adb94802e2007a407/nodebuilder/p2p/bootstrap.go#L45-L64
But internally, it's almost the same functions called. Do I revert? or we keep it until we have an issue?
Overview
Will be used to parse bootstrappers from a comma separated list.
Checklist