Skip to content

Extract protocol classes into nucypher-core#2802

Merged
KPrasch merged 22 commits intonucypher:mainfrom
fjarri:core
Oct 16, 2021
Merged

Extract protocol classes into nucypher-core#2802
KPrasch merged 22 commits intonucypher:mainfrom
fjarri:core

Conversation

@fjarri
Copy link
Contributor

@fjarri fjarri commented Sep 21, 2021

This PR attempts to extract protocol logic (that can be used e.g. in a JS implementation of nucypher) into a Rust module.

At first, to speed up the process and get a better idea of the API surface, I will just be extracting stuff to a separate Python module (nucypher/core.py).

Changes:

  • Moved to core.py:
    • MessageKit
    • HRAC
    • AuthorizedKeyFrag
    • TreasureMap/EncryptedTreasureMap
    • ReencryptionRequest/ReencryptionResponse
    • RetrievalKit
    • Arrangement (and added ArrangementResponse)
    • Revocation
    • node metadata is formalized as NodeMetadata
    • added MetadataRequest and MetadataResponse protocol objects
  • Using a single InvalidSignature exception (from crypto.signing). The ones from cryptography and the internal alias in Learner are removed.
  • Disbanded Character.verify_from(). The decrypting part of it is now a method in MessageKit.
  • Moved the verification part of Character.verify_from() to Learner. Fixes Character and Learner both have a verify_from() method that raises different exceptions with the same name #2203
  • Moved kfrag authorization check to AuthorizedKeyFrag

ABI changes (will need to bump serialized versions):

  • Storing the timestamp as epoch in Arrangement
  • Replaced CS constants in MessageKit with simple byte constants
  • Wrapped the MessageKit in the serialized RevocationOrder into VariableLengthBytestring (same as it is already done in TreasureMap) to get rid of the hardcoded encrypted size
  • Removed blockchain signature from EncryptedTreasureMap (it was unused)
  • Bundled policy_encrypting_key in TreasureMap (fixes Merge policy encrypting key into the treasure map #2792)
  • Node metadata versioning is now managed through Versioned, so the corresponding variables (TEACHER_VERSION, LEARNER_VERSION), the related machinery, and tests were removed.

Other public changes:

  • Removed teacher_version collector from Prometeus - it is not used in the dashboard. It is possible to add NodeMetadata.version_string() as an equivalent.

More ABI changes to be added in #2809.

fjarri added a commit to fjarri/nucypher that referenced this pull request Sep 21, 2021
@fjarri fjarri force-pushed the core branch 2 times, most recently from 971565b to fba7477 Compare September 21, 2021 23:56
fjarri added a commit to fjarri/nucypher that referenced this pull request Sep 22, 2021
@fjarri fjarri force-pushed the core branch 4 times, most recently from edb7a37 to dd36f24 Compare September 22, 2021 01:07
@fjarri fjarri added the API label Sep 22, 2021
@fjarri fjarri force-pushed the core branch 17 times, most recently from 68c9dc1 to ae4214c Compare September 28, 2021 22:39
fjarri added a commit to fjarri/nucypher that referenced this pull request Sep 29, 2021
@fjarri fjarri force-pushed the core branch 3 times, most recently from 45d2c54 to bd57ad7 Compare September 30, 2021 01:24
@fjarri
Copy link
Contributor Author

fjarri commented Oct 15, 2021

Updated the newsfragment; should it be misc, or is it rather removal?

@KPrasch
Copy link
Member

KPrasch commented Oct 15, 2021

Updated the newsfragment; should it be misc, or is it rather removal?

Either will do. I say misc.

def _collect_internal(self) -> None:
# info
base_payload = {'app_version': nucypher.__version__,
'teacher_version': str(self.ursula.TEACHER_VERSION),
Copy link
Member

Choose a reason for hiding this comment

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

Was this tested with prometheus?

Copy link
Member

Choose a reason for hiding this comment

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

I guess there is a follow-up PR to come - but want to stress that this should be tested either as part of this PR or the next.

Copy link
Member

Choose a reason for hiding this comment

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

👍🏻 Agreed. A follow up is already open #2809 - It can be included there. In any case we'll definitely stage these changes before pressing a release.

Copy link
Member

@derekpierre derekpierre left a comment

Choose a reason for hiding this comment

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

🎸

@KPrasch KPrasch merged commit 140508f into nucypher:main Oct 16, 2021
@fjarri fjarri deleted the core branch October 16, 2021 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants