Blue oyster mushrooms: Evolving node validity, optimization, domain separation, and migration. by jMyles · Pull Request #1881 · nucypher/nucypher

jMyles · 2020-04-13T21:07:33Z

Contains the following PRs:

Closes issues:

Closes Increment LearningLoop version and see what happens #1871: Bumped Learning Loop version from 1 to 2.
Closes Test with a hard-coded, versioned node metadata bytestring #1869: See new tests in tests/integration/learning/test_learning_versions.py.
Closes Relocate nucypher.datastore.keypairs.py to nucypher.crypto #2236: Relocate keypairs.py from datastore to crypto.
Closes Weirdness in network/domains mapping #1580: For the moment, networks == domains
Closes Problem deserializing domains in node metadata #1843: Handles bad domains from stored node metadata
Closes Invalid node bytes encountered in node storage #1623: Handles bad node metadata from storage
Closes Handle missing wallet address on TLS certificates #1820: Handle missing certificate checksum address pseudonym

Issues to address:

KPrasch · 2020-08-20T15:31:05Z

I'd like to suggest that we use the "scope hammer" for Issues regarding migration and include them in another PR.

KPrasch · 2020-09-23T19:22:17Z

nucypher/utilities/seednodes.py

-            uris.extend(hardcoded_uris)
-
-    return uris
+# TODO: This module seems unused


KPrasch

Turned into a very nice cleanup. 🤠

In it goes.

nucypher/blockchain/eth/actors.py

fjarri · 2020-09-23T20:42:00Z

nucypher/network/nodes.py

    __DEFAULT_MIDDLEWARE_CLASS = RestMiddleware

    LEARNER_VERSION = LEARNING_LOOP_VERSION
+    LOWEST_COMPATIBLE_VERSION = 2   # Disallow versions lower than this


Feels like a semantic version tuple would be more intuitively understandable here.

YES! I though something like that but also had second thoughts regarding how to encode this efficiently.

This raises two questions from me:

What does a minor vs major bump mean?

How and when does the version get updated?

Both good questions. The current design in main implicitly assumes all versions are going to be back-wards compatible, in the sense that if you detect a node from a past version, no special handling of the metadata is performed. There are cases, like our particular bump from version 1 to version 2, where this is not desirable (as this is a "major" change in semver parlance), but I certainly can imagine other cases where we bump the version and at the same time try to maintain compatibility with the old version (a "minor" change). The approach taken in this PR is very simplistic, and I think there's room for improvement, but at least ensures that no metadata from version 1 will linger on the network.

🤔 Interesting - I suggest we open an Issue to track and leave it for a future PR

How can we keep a whole other Ursula class accessible? What's the mechanism here?

Kind of like what's happening in this test:
https://github.com/nucypher/bytestringSplitter/blob/master/tests/test_kwargifiers.py#L127

The "EnergyDrink" class, in this case being the "future Ursula with new methods and attributes"

What I meant is that you'll have to have the V1 Ursula class defined somewhere. Which is huge, and has links to other parts of the codebase. Feels like it would be easier to convert whatever you deserialized to the current Ursula.

Well yeah... it depends on where the future takes us. If the V1 can be the super class of the V2, then it's not so bad, we just change/add some methods.

If it's a total brand new distinct class, then it's less fun.

I just realized, something similar to this situation is already happening in the case of TreasureMaps vs SignedTreasureMaps.

Two distinct classes with different bytestring de/serializations, one a subclass of the other.

here

…rent domain Demonstrates #1843

… nodes from other domains

Co-authored-by: Derek Pierre <derek.pierre@gmail.com>

… class with lower-level crypto.api

nucypher/characters/lawful.py

nucypher/crypto/api.py

nucypher/network/nodes.py

nucypher/utilities/prometheus/collector.py

derekpierre · 2020-09-25T14:46:45Z

tests/integration/learning/test_discovery_phases.py

        # The number of nodes having the map is approximately the number you'd expect from full utilization of Alice's publication threadpool.
        # TODO: This line fails sometimes because the loop goes too fast.
-        assert len(nodes_that_have_the_map_when_we_unblock) == pytest.approx(policy.publishing_mutex._block_until_this_many_are_complete, .2)
+        # assert len(nodes_that_have_the_map_when_we_unblock) == pytest.approx(policy.publishing_mutex._block_until_this_many_are_complete, .2)


Do we want to do any follow-up here?

It's in issue #2016

Co-authored-by: Derek Pierre <derek.pierre@gmail.com>

derekpierre

🎸

jMyles mentioned this pull request Apr 13, 2020

Even lazier node instantiation. #1868

Merged

KPrasch added this to the Before activating StakersEscrow milestone Apr 14, 2020

jMyles force-pushed the blue-oyster-mushrooms branch from dfb1e5d to e8338e9 Compare April 21, 2020 19:45

mswilkison linked an issue Apr 22, 2020 that may be closed by this pull request

What's a "network" in nucypher? #1564

Closed

mswilkison removed a link to an issue Apr 22, 2020

What's a "network" in nucypher? #1564

Closed

KPrasch assigned jMyles Apr 30, 2020

mswilkison added the Epic 📖 Upstream branches with a project focus label May 11, 2020

KPrasch changed the base branch from master to main July 21, 2020 00:44

jMyles mentioned this pull request Aug 19, 2020

Firstula: start lonely, but learn when learned about #2186

Closed

cygnusv added the BlueOyster🍄 label Aug 21, 2020

KPrasch mentioned this pull request Sep 1, 2020

Are we done with multiple-domain discovery? #2144

Closed

cygnusv force-pushed the blue-oyster-mushrooms branch 2 times, most recently from 21179e3 to 7f39e90 Compare September 14, 2020 11:35

cygnusv force-pushed the blue-oyster-mushrooms branch 2 times, most recently from 7fba47c to 0fa885a Compare September 20, 2020 15:22

KPrasch force-pushed the blue-oyster-mushrooms branch from 0fa885a to cf8913f Compare September 21, 2020 22:36

cygnusv force-pushed the blue-oyster-mushrooms branch from cf8913f to 395135f Compare September 22, 2020 07:56

cygnusv changed the title ~~[WIP] Blue oyster mushrooms: Evolving node validity, optimization, domain separation, and migration.~~ Blue oyster mushrooms: Evolving node validity, optimization, domain separation, and migration. Sep 23, 2020

cygnusv force-pushed the blue-oyster-mushrooms branch from d2a6ec6 to 100175c Compare September 23, 2020 18:23

KPrasch requested review from KPrasch, cygnusv, derekpierre, fjarri and vepkenez September 23, 2020 18:26

KPrasch reviewed Sep 23, 2020

View reviewed changes

nucypher/utilities/seednodes.py

uris.extend(hardcoded_uris)

return uris

# TODO: This module seems unused

Copy link

Member

KPrasch Sep 23, 2020

Choose a reason for hiding this comment

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

#2282

KPrasch approved these changes Sep 23, 2020

View reviewed changes

fjarri reviewed Sep 23, 2020

View reviewed changes

nucypher/blockchain/eth/actors.py Outdated Show resolved Hide resolved

cygnusv mentioned this pull request Sep 23, 2020

Ursula 'Known Nodes' value never decreases, and carries across networks #2292

Closed

fjarri reviewed Sep 23, 2020

View reviewed changes

cygnusv and others added 20 commits September 24, 2020 20:27

Include test for hardcoded version 2 node metadata bytestrings

14c6307

Some unit tests reorganization

064972e

Relocate keypairs.py from datastore to crypto

0fb2cad

Some janitorial stuff

f42de3a

When serializing nodes to disk, just use bytes, not hex (the default)

f86eda9

Improve files organization by TemporaryFileBasedNodeStorage

fcd7edb

Failing test showing that currently we learn about nodes from a diffe…

d4ac7ce

…rent domain Demonstrates #1843

Ignore restored nodes from disk that belong to a different domain

7a1907f

When restoring nodes from disk, let's just complain once if there are…

c897a20

… nodes from other domains

Handle invalid metadata bytes in node storage. Unit tests.

279f07c

Simplify approach to node encoders/decoders in NodeStorage & Sons

7d330ea

Don't return bool on Storage initialization. It wasn't used anyway.

ff10f2e

Don't call initialize() at init time for temp file NodeStorages

4f60c61

Co-authored-by: Derek Pierre <derek.pierre@gmail.com>

Additional checks and handling of invalid node certs; Unify exception…

adf1c84

… class with lower-level crypto.api

Handle invalid node cert during node validation

c0313e8

Handle invalid node cert during availability tracking

0f63103

Fix imports

538aa79

Comment flaky assertion on test_discovery_phases. See #2016

3d65024

Utility test to obtain versioned Ursulas' metadata hexstrings

f96e82d

Address RFCs on #1881

47e01d6

cygnusv force-pushed the blue-oyster-mushrooms branch from 2254ee3 to 47e01d6 Compare September 24, 2020 18:29

Pending change to single-domain from the Trezor PR

df06e3e

derekpierre reviewed Sep 25, 2020

View reviewed changes

cygnusv mentioned this pull request Sep 25, 2020

Flaky Tests List #2016

Closed

Apply suggestions from code review

ad696b5

Co-authored-by: Derek Pierre <derek.pierre@gmail.com>

derekpierre approved these changes Sep 25, 2020

View reviewed changes

vepkenez approved these changes Sep 25, 2020

View reviewed changes

KPrasch merged commit 9b45c08 into main Sep 25, 2020

cygnusv mentioned this pull request Sep 29, 2020

Blue Oyster Mushrooms aftertaste #2307

Merged

KPrasch deleted the blue-oyster-mushrooms branch January 22, 2021 19:05

Conversation

jMyles commented Apr 13, 2020 • edited by KPrasch Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KPrasch commented Aug 20, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KPrasch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vepkenez Sep 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fjarri Sep 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

derekpierre left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jMyles commented Apr 13, 2020 •

edited by KPrasch

Loading

vepkenez Sep 24, 2020 •

edited

Loading

fjarri Sep 24, 2020 •

edited

Loading