fix: loop counter for writing multiaddresses in enr

[richard-ramos]

Description

Writing multiaddresses on the ENR multiaddr key worked only when there was more than one multiaddress field that should be stored on that field. Most of the time that wasn't a problem, specially since desktop does not use websocket addresses and always has at least 2 circuit relay addresses

[status-im-auto]

Jenkins Builds

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	0e74ea5	#1	2023-09-01 21:55:19	~1 min	linux	📦deb
✔️	0e74ea5	#1	2023-09-01 21:56:03	~2 min	nix-flake	📄log
✔️	0e74ea5	#1	2023-09-01 21:56:54	~2 min	tests	📄log
✔️	0e74ea5	#1	2023-09-01 21:57:03	~2 min	tests	📄log
✔️	0e74ea5	#1	2023-09-01 21:57:57	~3 min	android	📦tgz
✔️	0e74ea5	#1	2023-09-01 21:58:23	~4 min	ios	📦tgz
✔️	0847007	#2	2023-09-08 17:32:49	~1 min	tests	📄log
✔️	0847007	#2	2023-09-08 17:33:06	~1 min	linux	📦deb
✔️	0847007	#2	2023-09-08 17:33:19	~1 min	tests	📄log
✔️	0847007	#2	2023-09-08 17:33:23	~1 min	nix-flake	📄log
✔️	0847007	#2	2023-09-08 17:35:07	~3 min	android	📦tgz
✔️	0847007	#2	2023-09-08 17:35:36	~4 min	ios	📦tgz

[chaitanyaprem]

LGTM

[harsh-98]

• if the successInd is found, we are trying to write the multiAddress field to the local node again.
  I think this logic is enough?

		for i := len(multiaddrs) ; i >= 0; i-- {
			err = writeMultiaddressField(localnode, multiaddrs[0:i])
			if err == nil {
				couldWriteENRatLeastOnce = true
				successIdx = i
				return nil 
			} else {
				failedOnceWritingENR = true
			}
		}

		if failedOnceWritingENR {
			return errors.New("could not write new ENR")
		}

failedOnceWritingENR is still needed to prevent error in case multiaddrs array is empty.

• Secondly, i goes from n to 0, shouldn't it go from n to 1. The last iteration will provide 0 len array to writemultiAddressField.

i found some suggestions.

[richard-ramos]

@harsh-98: * if the successInd is found, we are trying to write the multiAddress field to the local node again.

The reason why this is done this way is because a failure to write the ENR will still modify the multiaddrs fields, making the ENR unusable, so successInd is used to revert the multiaddrs field back to the last usable state. This could probably be improved by having writeMultiaddressField do the revert instead of having it done in this file.

I can do this in a separate PR:

i.e. - instead of doing it this way:

defer func(){
	if e := recover(); e != nil {
		// Deleting the multiaddr entry, as we could not write it succesfully
		localnode.Delete(enr.WithEntry(MultiaddrENRField, struct{}{}))
		err = errors.New("could not write enr record")
	}
}()

to instead do:

var ogMultiaddrValue []byte
if err := node.Record().Load(enr.WithEntry(MultiaddrENRField, &ogMultiaddrValue)); err != nil {
	if !enr.IsNotFound(err) {
		return err
	}
}

defer func(){
	if e := recover(); e != nil {
                // restoring to original value
		localnode.Set(enr.WithEntry(MultiaddrENRField, ogMultiaddrValue))
		err = errors.New("could not write enr record")
	}
}()

[richard-ramos]

@harsh-98: Secondly, i goes from n to 0, shouldn't it go from n to 1. The last iteration will provide 0 len array to writemultiAddressField.

sounds good. I'll fix this