skip returning error is an identity address on chain cannot be parsed #1136
Conversation
vishalchangrani
requested review from
huitseeker,
zhangchiqing,
Kay-Zee and
yhassanzadeh13
| @@ -536,7 +536,7 @@ func (n *Node) UpdateAllowList(identities flow.IdentityList) error { | |||
| for i, identity := range identities { | |||
| allowlist[i], err = PeerAddressInfo(*identity) | |||
| if err != nil { | |||
| return fmt.Errorf("could not generate address info: %w", err) | |||
| n.logger.Err(err).Str("identity", identity.String()).Msg("could not generate address info") | |||
There was a problem hiding this comment.
Not returning an error but rather logging it.
There was a problem hiding this comment.
I think this would cause allowlist to have nil object.
There was a problem hiding this comment.
yup, changed.
There was a problem hiding this comment.
Thanks.
Another comment is that I wonder if we could do this check earlier?
Since the validation of the peer address does not depend on any state, we could technically move the validation earlier. We just need to figure out the responsibility of who should validate the identity here. libp2p2? or overlay?
libp2p receives this list from overlay.Identity, should we let overlay filter out peers with invalid address?
There was a problem hiding this comment.
yeah, that is not a bad idea. @smnzhu is actually replacing the ids with an ID provider so I will add this validation over there.
Codecov Report
@@ Coverage Diff @@
## master #1136 +/- ##
==========================================
- Coverage 53.11% 53.11% -0.01%
==========================================
Files 322 322
Lines 21829 21830 +1
==========================================
Hits 11594 11594
- Misses 8662 8663 +1
Partials 1573 1573
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
am deploying this on Canary (by backporting it to |
Canary network nodes are currently down because of the following error:
There was some recent Flow port testing that was done which submitted an identity with an address which was missing the port field.