Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore_: bump go-waku to change datatype of waku2 field in ENR to byte array #5882

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

richard-ramos
Copy link
Member

Allows decoding ENRs whose waku2 field contains 0x00. For example, see enr:-LW4QL-wGVQsfol6Bc4Twnnl2LkHu2oTWD-RIm2FDlG8c4aYIM6bKsMnN_tjP-rH8SddEICwcxrslhRlcnPaRnHDAP0BgmlkgnY0gmlwhMCoAAGKbXVsdGlhZGRyc4CCcnOTAAAIAAAAAQACAAMABAAFAAYAB4lzZWNwMjU2azGhAjn6QX7KNxZGvJf9trmXLjQcsbnEdEIUKG_VPgDymcUog3RjcILqYIN1ZHCCIyiFd2FrdTIA in enr-viewer.com

Otherwise the following error will be shown: rlp: non-canonical integer (leading zero bytes) for uint8

EDIT: See also discussion here on why it should be a byte array instead to allow extending the field to support more than 8 protocols https://discord.com/channels/1110799176264056863/1276567145785987195/1287897621184380938

For testing purposes behavior should be the same as it is now, this change is just so go-waku and nwaku have the same behavior

Requires:

@status-im-auto
Copy link
Member

status-im-auto commented Sep 25, 2024

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ e91568c #1 2024-09-25 21:18:40 ~2 min tests-rpc 📄log
✔️ e91568c #1 2024-09-25 21:19:33 ~3 min linux 📦zip
✔️ e91568c #1 2024-09-25 21:20:01 ~4 min ios 📦zip
✔️ e91568c #1 2024-09-25 21:20:28 ~4 min android 📦aar
✔️ e91568c #1 2024-09-25 21:47:58 ~32 min tests 📄log

@codecov-commenter
Copy link

codecov-commenter commented Sep 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.04%. Comparing base (d794e43) to head (e91568c).
Report is 55 commits behind head on develop.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #5882      +/-   ##
===========================================
+ Coverage    45.90%   46.04%   +0.13%     
===========================================
  Files          884      884              
  Lines       157168   157166       -2     
===========================================
+ Hits         72150    72360     +210     
+ Misses       76925    76500     -425     
- Partials      8093     8306     +213     
Flag Coverage Δ
functional 11.57% <ø> (?)
unit 45.62% <ø> (-0.29%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 124 files with indirect coverage changes

@adklempner adklempner requested a review from a team October 8, 2024 21:10
@adklempner adklempner merged commit 832db22 into develop Oct 9, 2024
11 of 12 checks passed
@adklempner adklempner deleted the enr-byte branch October 9, 2024 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants