bug fix in +nap:to #1128
bug fix in +nap:to #1128
Conversation
There was a problem hiding this comment.
@yosoyubik Thank you! This is good work and an important fix. I requested a minor cleanup in the tests, but this looks good and should be ready to merge afterward.
@joemfb I'm not sure what the proper git procedure is here: edit the PR to be against hotfix? Seems like a good candidate for an OTA update. I'm happy to shepherd this in however you'd like.
@vvisigoth @philipcmonk The queue bug fixed by this PR could be the cause of our long-standing one-way connectivity issues (https://github.com/urbit/arvo/issues/717, https://github.com/urbit/arvo/issues/554, #170). Ames performs ~(nap to ...) on its packet queue when acking a packet in +bine:pu.
I suspect it's worth writing a reproduction case by constructing a three-element packet queue, getting Ames to ack a packet, and then seeing if we can confirm downstream OWC. We could then apply this fix and confirm the lack of OWC.
Woah It looks like this function is only used twice: in ames and eyre. Seems unlikely that anything is depending on its brokenness, hotfix is probably a good idea. |
|
Agreed, the right thing is to edit a PR to target the appropriate branch. It seems like |
|
Thank you guys! I didn't expect this to be useful at all... There I was just making a small tic-tac-toe app and look at this :D I have just edited the PR to target hotfix, let me know if something else is needed. |
Fixes #1100 (a bug in +nap:to affecting three-item queues)
Unit tests added in /tests/sys/hoon/qeu