Skip to content

TreasureMap con KFrags fixes, mostly with compatibility and splitting previous versions #2719

Merged
jMyles merged 18 commits intonucypher:chili-con-carnefrom
jMyles:chili-con-carnations
Jun 9, 2021
Merged

TreasureMap con KFrags fixes, mostly with compatibility and splitting previous versions #2719
jMyles merged 18 commits intonucypher:chili-con-carnefrom
jMyles:chili-con-carnations

Conversation

@jMyles
Copy link
Copy Markdown
Contributor

@jMyles jMyles commented Jun 2, 2021

Type of PR:

  • Bugfix
  • Feature
  • Documentation
  • Other

Required reviews:

  • 1
  • 2
  • 3

Versions TreasureMaps, replaces some of @KPrasch's work within the class with the work @vepkenez had previously done to achieve very similar functionality. Also "brands" TreasureMaps - and distinctly, SignedTreasureMaps, for easy ascertainment across the network.

Updates depedencies.

@jMyles jMyles changed the title [WIP] TreasureMap con KFrags fixes, mostly with compatibility and splitting previous versions TreasureMap con KFrags fixes, mostly with compatibility and splitting previous versions Jun 2, 2021
@jMyles jMyles changed the base branch from main to chili-con-carne June 2, 2021 22:55
@jMyles jMyles requested review from KPrasch and fjarri June 3, 2021 18:15
@jMyles jMyles assigned vepkenez and unassigned vepkenez Jun 3, 2021
@jMyles jMyles requested a review from vepkenez June 3, 2021 18:16
fjarri added a commit to fjarri/nucypher that referenced this pull request Jun 3, 2021
@jMyles jMyles force-pushed the chili-con-carne branch from 6070910 to 2984265 Compare June 7, 2021 17:02
@jMyles jMyles force-pushed the chili-con-carnations branch from 19b7988 to 2b8c02e Compare June 7, 2021 17:16
@KPrasch
Copy link
Copy Markdown
Member

KPrasch commented Jun 7, 2021

The CI failure for gas estimation looks like it can be resolved with a simple rename, might as well include it in this PR.

Copy link
Copy Markdown
Member

@KPrasch KPrasch left a comment

Choose a reason for hiding this comment

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

Great advancement - Definitely good enough to merge into the epic. I even suggest reducing the amount of required approvals to two.

fjarri added a commit to fjarri/nucypher that referenced this pull request Jun 7, 2021
@jMyles jMyles force-pushed the chili-con-carnations branch from 95748d0 to 6d3cd88 Compare June 8, 2021 19:34
Copy link
Copy Markdown
Contributor

@fjarri fjarri left a comment

Choose a reason for hiding this comment

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

Approved with reservations to avoid blocking further work:

  • I am still not sure that the __new__ approach is the best here, I'll revisit the topic when we have another versioned class and have to copy-paste or generalize.
  • In fact, I am not even certain that object versioning as opposed to protocol interactions versioning is the way to go, but this PR does not close the way to the latter yet.
  • I think it is possible to make SignedTreasureMap valid on creation, and not after manually setting all the required fields with additional method calls - I'll try it out later.

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.

4 participants