[EPIC] Schabowski: Fixes for bugs revealed by Coinlist, EthCapeTown, and EthNY hackathons by jMyles · Pull Request #926 · nucypher/nucypher

jMyles · 2019-04-09T13:44:52Z

Chaperones: @jMyles

Establishes new epic process (Fixes Implement epic-v1 #925)
Documents epics in pre-release period
Corrects Character Control encoding (Fixes Some controller endpoints fail when trying to serialize output. #920)
Fixes Bob's multiple retrieval issues (Fixes Bob cannot retrieve the same data twice #833)
Corrects side-effects of --debug (Fixes Finnegan's Wake demo... only works with --debug. #941)

Named for Günter Schabowski, the somewhat hapless spokesperson for the Sozialistische Einheitspartei Deutschlands who announced on 9 Nov 1989 that travel restrictions from East to West Berlin had been lifted effective immediately - he was supposed to announce that they were going to be slowly lifted over several months. His announcement led to a surge of many thousands of people to the wall - many with hammers and chisels - and, a few hours later, into West Berlin for the first time in decades.

Runner-up name: Ginger, based on what @jMyles was drinking when he opened it.

codecov · 2019-04-09T14:02:58Z

Codecov Report

Merging #926 into master will increase coverage by 1.11%.
The diff coverage is 75.47%.

@@            Coverage Diff             @@
##           master     #926      +/-   ##
==========================================
+ Coverage   82.13%   83.25%   +1.11%     
==========================================
  Files          68       68              
  Lines        9081     9103      +22     
==========================================
+ Hits         7459     7579     +120     
+ Misses       1622     1524      -98

Impacted Files	Coverage Δ
nucypher/cli/characters/ursula.py	`70.93% <100%> (+0.14%)`	⬆️
nucypher/network/nodes.py	`84.82% <100%> (+0.02%)`	⬆️
nucypher/cli/main.py	`88.52% <50%> (-2.71%)`	⬇️
nucypher/network/server.py	`86.99% <60%> (-0.97%)`	⬇️
nucypher/characters/lawful.py	`90% <80.55%> (+0.34%)`	⬆️
nucypher/utilities/sandbox/blockchain.py	`90.64% <0%> (+0.71%)`	⬆️
nucypher/cli/characters/felix.py	`69% <0%> (+9%)`	⬆️
nucypher/characters/chaotic.py	`75% <0%> (+31.94%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 72977f0...a106af2. Read the comment docs.

cygnusv · 2019-04-25T10:27:24Z

Hey @jMyles, what about adding my fix to #245 here?
Here's the commit: a7d9d7c

KPrasch · 2019-04-30T17:58:36Z

I think #941 Will also be fixed in a parallel effort (#951)

cygnusv · 2019-05-31T18:35:34Z

@jMyles Just bumping this:

Hey @jMyles, what about adding my fix to #245 here?
Here's the commit: a7d9d7c

…against this Capsule before.

…e the WorkOrder.

…hese per #567.

…se file.

…el. A good-enough compromise for now I guess.

…t, on which test assertion relies.

…t spec.

KPrasch · 2019-07-01T16:04:50Z

tests/characters/test_bob_joins_policy_and_retrieves.py


    assert plaintext == delivered_cleartexts[0]

-    # FIXME: Bob tries to retrieve again


KPrasch · 2019-07-01T16:05:00Z

tests/cli/test_mixed_configurations.py


    # Run an Ursula amidst the other configuration files
-    run_args = ('ursula', 'run', '--dry-run',
+    run_args = ('ursula', 'run', '--dry-run', '--interactive',


KPrasch · 2019-07-01T16:54:26Z

tests/fixtures.py

-    enrico = Enrico(policy_encrypting_key=enacted_federated_policy.public_key)
-    message_kit, _signature = enrico.encrypt_message(b"Welcome to the flippering.")
-    return message_kit, enrico
+    class _CapsuleSideChannel:


Another Side Channel... We are accumulating them!

This is a replacement; we are net zero side channels here. :-)

KPrasch · 2019-07-01T16:55:27Z

nucypher/network/server.py

                # Async Sentinel
                except Exception as e:
-                    log.critical(str(e))
+                    log.critical(f"This exception really needs to be handled differently: {e}")


OK..... haha - Perhaps an Issue #?

It's not a specific issue; it's just that any exception that makes it this far needs to be handled differently.

tuxxy · 2019-07-01T23:51:11Z

nucypher/characters/lawful.py

+
+        if already_retrieved:
+            if cache:
+                must_do_new_retrieval = False


nucypher/characters/lawful.py

tuxxy · 2019-07-02T00:10:14Z

tests/characters/control/test_web_control.py

+    # break Bob's retrieve endpoint.
+    # convo starts here: https://ptb.discordapp.com/channels/411401661714792449/411401661714792451/564353305887637517
+
+    bad_label = '516d593559505355376d454b61374751577146467a47754658396d516a685674716b7663744b376b4b666a35336d'


tuxxy

👍

cygnusv

Some minor comments, but overall all good!

cygnusv · 2019-07-02T09:38:30Z

tests/characters/control/test_web_control.py

+
+
+@pytest.mark.skip("920")
+def test_label_whose_b64_representation_is_invalid_utf8():


Why is this still skipped?

That's a mistake.

cygnusv · 2019-07-02T09:43:20Z

nucypher/characters/lawful.py

        # Users may decide to inject some market strategies here.
        #
-        # TODO: #289
+        # TODO: 289


I think @KPrasch and you have been fighting a silent war: he puts the number sign in TODOs and you remove it.

https://getyarn.io/yarn-clip/b855c713-3d55-45c6-8c58-feb088baba55

nucypher/network/nodes.py

cygnusv · 2019-07-02T09:53:16Z

tests/fixtures.py

+
+        def __call__(self):
+            enrico = Enrico(policy_encrypting_key=enacted_federated_policy.public_key)
+            message = "Welcome to the flippering.".format(len(self.messages)).encode()


why are you doing .format(len(self.messages))? It seems to have no effect.

cygnusv · 2019-07-02T09:58:24Z

@jMyles Can you add a TODO in this line:

nucypher/nucypher/characters/lawful.py

Line 650 in c613bcd

ursula = self.known_nodes[node_id]

, referencing issue #999 ?

vepkenez · 2019-07-03T21:19:37Z

I love this PR. I have personally been affected by many of these issues and I'm glad to see them gone! Yay! 💯 👍

jMyles force-pushed the epic-schabowski branch from b63afec to b18a22d Compare April 9, 2019 13:48

jMyles force-pushed the epic-schabowski branch from df525e2 to 0ba62b5 Compare April 12, 2019 11:44

jMyles changed the title ~~[WIP] [EPIC] Schabowski~~ [WIP] [EPIC] Fixes for bugs revealed by Coinlist and EthCapeTown hackathons Apr 30, 2019

jMyles force-pushed the epic-schabowski branch 3 times, most recently from 7f76846 to 55f6d53 Compare May 16, 2019 16:20

jMyles force-pushed the epic-schabowski branch 2 times, most recently from c9745d0 to ec682f4 Compare June 9, 2019 21:56

jMyles force-pushed the epic-schabowski branch 2 times, most recently from c1804dc to 9997dd2 Compare June 22, 2019 05:14

jMyles changed the title ~~[WIP] [EPIC] Fixes for bugs revealed by Coinlist and EthCapeTown hackathons~~ [EPIC] Fixes for bugs revealed by Coinlist and EthCapeTown hackathons Jun 29, 2019

jMyles changed the title ~~[EPIC] Fixes for bugs revealed by Coinlist and EthCapeTown hackathons~~ [EPIC] Schabowski: Fixes for bugs revealed by Coinlist, EthCapeTown, and EthNY hackathons Jun 29, 2019

jMyles requested review from KPrasch and cygnusv and removed request for cygnusv June 29, 2019 02:30

jMyles added 11 commits June 28, 2019 20:10

Test showing 920.

694ba1b

First step: let's stop caring for the moment whether we've retrieved …

4c0d51f

…against this Capsule before.

OK, start with a cache argument in generate_work_orders.

c06cf4b

...and we'll pass the cache argument down from retrieve.

a3df1d0

Now, we'll work in the matter of whether to retrieve anew.

ab6dfa7

Instead of raising here, give 'em the ole' 404.

e393509

Test showing Bob's second retrieval.

3e94e91

Test showing 892.

eab64bd

With caching in place, we can again apply the logic of whether to sav…

18fe0d6

…e the WorkOrder.

Always set the correctness keys; new retrieval or not.

aa1392a

Adding --log-level as a CLI option. See #1018.

37c9b75

jMyles added 10 commits June 28, 2019 20:10

Logging phantom node situation, though we do need to full-on bucket t…

3f59475

…hese per #567.

A different way to assert work_order similarity.

8b14f5d

Make UrsulaCommandProtocol opt-in. Closes #1087.

820ae2c

Assigning each instance's IP address as its rest port in docker compo…

e182d46

…se file.

Skipping test until #920 is closed.

a88d8c4

Cleaning up retrieve() by consolidation a branch.

06a75d1

Adding a block to catch NotFound, in case Ursula 404's.

64df431

Only use SimpleObserver if the user has expressly specified a log lev…

2b6f6c6

…el. A good-enough compromise for now I guess.

Adding --interactive for the moment in order to display command promp…

54249eb

…t, on which test assertion relies.

Our Ursula from config is also supposed to be interactive per the tes…

523fde5

…t spec.

jMyles force-pushed the epic-schabowski branch from 8fb06be to 523fde5 Compare June 29, 2019 03:13

A few new places where capsule_side_channel needs to be called.

c613bcd

KPrasch reviewed Jul 1, 2019

View reviewed changes

KPrasch approved these changes Jul 1, 2019

View reviewed changes

tuxxy reviewed Jul 2, 2019

View reviewed changes

tuxxy approved these changes Jul 2, 2019

View reviewed changes

cygnusv approved these changes Jul 2, 2019

View reviewed changes

jMyles added 2 commits July 3, 2019 13:51

Adding message number to side channel flow.

afe2666

Updating test showing that #920 is fixed.

a57affc

vepkenez self-requested a review July 3, 2019 21:19

vepkenez approved these changes Jul 3, 2019

View reviewed changes

jMyles and others added 2 commits July 3, 2019 16:55

Resetting side channel between two of the tests.

189a873

Apply suggestions from code review

a106af2

cygnusv merged commit a2cdafd into master Jul 4, 2019

jMyles deleted the epic-schabowski branch August 22, 2019 13:11


		assert plaintext == delivered_cleartexts[0]

		# FIXME: Bob tries to retrieve again



		@pytest.mark.skip("920")
		def test_label_whose_b64_representation_is_invalid_utf8():

Conversation

jMyles commented Apr 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Apr 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cygnusv commented Apr 25, 2019

Uh oh!

KPrasch commented Apr 30, 2019

Uh oh!

cygnusv commented May 31, 2019

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tuxxy left a comment

Choose a reason for hiding this comment

Uh oh!

cygnusv left a comment

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cygnusv commented Jul 2, 2019

Uh oh!

vepkenez commented Jul 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jMyles commented Apr 9, 2019 •

edited

Loading

codecov bot commented Apr 9, 2019 •

edited

Loading