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

jael: give new private key to subscribers on %keys new-event #5532

Merged
merged 39 commits into from Jan 12, 2022

Conversation

yosoyubik
Copy link
Contributor

@yosoyubik yosoyubik commented Jan 8, 2022

This is built on top of #5496 using the latest snapshot. A solid pill is also included—a new pill should be pushed to bootstrap.urbit.org before any L2 ships are booted, otherwise they might end up in a situation where the eth-log that includes the new private key comes before the OTA that has this fix, which will make them disconnected from the network, before the fix for #5517 is applied, and a manual intervention would be needed.

This also (from #5496) switches /app/azimuth to use a snapshot of the Azimuth state, instead of relying on eth-logs, which will be applied on-load, and will bring all ships of the network up to date, fixing the incomplete state found in #5518

Couple of tests performed to validate that this works:

  • Boot from a pill that contains this fix, after going through the invite flow on Bridge
  • Boot from the current pill, and downloading an OTA that contains this fix.
  • Applied this PR to ships that lost connectivity after switching to the new private key, when the L2 batch that contained the %keys event was processed by /app/azimuth

This PR has been tested and deployed to ~roller-dozzod-dozzod and ~sordem (both online) and several L2 ships.

This adds support for handling cases where the send-batch thread failed,
mainly among them, a thread crash. One of the events that causes this
behavior is a ver low gas price for this L1 transaction.

Here we add support for manually bumping the price for such transaction,
and for increasing the default fallback gas-price, together with discarding
any malformed batch from the sending queue.
@yosoyubik
Copy link
Contributor Author

I also booted a comet from the current pill and |OTAed from ~sordem (that has this PR deployed) and it got merged successfully—I could |hi several ships, both L1 and L2

@yosoyubik
Copy link
Contributor Author

yosoyubik commented Jan 10, 2022

I missed this looking at the backlog, but noticed this after receiving the OTA:

ping: strange state [~marzod ~ u=[rift=4 ship-state=[%waiting until=~2022.1.10..15.10.01..15bd]]]
ping: strange state [~zod ~ u=[rift=2 ship-state=[%waiting until=~2022.1.10..15.10.01..1ab9]]]

PS: I can |hi them just fine, and also other ships, so not sure how concerning this is.

@philipcmonk
Copy link
Contributor

ping: strange state is fine

Copy link
Contributor

@philipcmonk philipcmonk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it should work

Comment on lines 738 to 754
=? lyf.own
?& =(our ship.i.udiffs)
=- (public-keys:feel original-pos %diff ship.i.udiffs u.a-diff)
?. ?& =(our ship.i.udiffs)
?=(%keys -.u.a-diff)
(~(has by jaw.own) life.to.u.a-diff)
==
life.to.u.a-diff
(public-keys:feel original-pos %diff ship.i.udiffs u.a-diff)
this-su
:: if this about our keys, and we already know these, start using them
::
=. lyf.own life.to.u.a-diff
:: notify subscribers (ames) to start using our new private keys
::
(exec yen.own [%give %private-keys [lyf jaw]:own])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work? I think so, but the pattern feels weird since it puts two conflicting copies of this-su in our subject, like as though it needs a public-keys:feel:- to make sure we're using the right one.

I'd probably do something like:

      =?    this-su
          ?&  =(our ship.i.udiffs)
              ?=(%keys -.u.a-diff)
              (~(has by jaw.own) life.to.u.a-diff)
          ==
        ::  if this about our keys, and we already know these, start using them
        ::
        =.  lyf.own  life.to.u.a-diff
        ::  notify subscribers (ames) to start using our new private keys
        ::
        (exec yen.own [%give %private-keys [lyf jaw]:own])
      ::
      (public-keys:feel original-pos %diff ship.i.udiffs u.a-diff)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I tested it with a couple of L2 ships, but agree that your suggestion looks better—I deployed it 432d967 on ~sordem and also booted a L2 ship from a the pill and checking that communication with other ships works fine before and after switching to the new private keys.

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.

None yet

2 participants