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

[WIP] Add facility to assure backwards compatibility for objects that need to remain backwards compatibility #75

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

derekpierre
Copy link
Member

Type of PR:

  • Bugfix
  • Feature
  • Documentation
  • Other

Required reviews:

  • 1
  • 2
  • 3

What this does:

High-level idea of the changes introduced in this PR.
List relevant API changes (if any), as well as related PRs and issues.

We need a facility to assure that some objects remain backwards compatible. Examples of where this is needed is for protocol objects that may be persisted and then processed by later versions of the nucypher-core library eg. MessageKits i.e. encrypted data. These protocol types need to be "perpetually" backwards compatible. If any backwards incompatible changes are needed for types that are supposed to always remain backwards compatible, then a brand new type should be created/added.

There are legacy protocol object types that have already made major version changes that should have remained backwards compatible; it's possible that it was versioned during development but earlier versions were never released.

Existing legacy objects that probably need to be specified as "must remain backwards compatible" on such a facility is established:

  • SessionStaticKey (persisted in Coordinator contract)
  • AuthorizedKeyFrag, EncryptedKeyFrag (persisted in TreasureMap)
  • MessageKit (persisted in storage/side channel)
  • TreasureMap, AuthorizedTreasureMap, EncryptedTreasureMap (persisted in storage/side channel).

Issues fixed/closed:

  • Fixes #...

Why it's needed:

Explain how this PR fits in the greater context of the NuCypher Network.
E.g., if this PR address a nucypher/productdev issue, let reviewers know!

Notes for reviewers:

What should reviewers focus on?
Is there a particular commit/function/section of your PR that requires more attention from reviewers?

…objects that need to remain backwards compatible for the long-term eg. objects that get persisted to some type of storage.
@derekpierre derekpierre mentioned this pull request Aug 20, 2023
7 tasks
@codecov-commenter
Copy link

Codecov Report

Merging #75 (751f572) into main (d1a9cad) will increase coverage by 0.26%.
The diff coverage is 92.30%.

@@            Coverage Diff             @@
##             main      #75      +/-   ##
==========================================
+ Coverage   21.22%   21.48%   +0.26%     
==========================================
  Files          16       16              
  Lines        3176     3188      +12     
==========================================
+ Hits          674      685      +11     
- Misses       2502     2503       +1     
Files Changed Coverage Δ
nucypher-core/src/versioning.rs 63.76% <92.30%> (+2.65%) ⬆️

@piotr-roslaniec
Copy link
Contributor

piotr-roslaniec commented Aug 25, 2023

I like this proposal as-it-is, I think it solves our short & medium-term needs. For the long-term, I think there could be something clever we could do with Rust traits and Adopter pattern to enforce backward compatibility (or lack thereof) for protocol objects. See the rough sketch below:

// Assuming ProtocolObjectV1 and ProtocolObjectV2 are defined somewhere

// Error that may occur during migration
pub struct MigrationError;

// Trait for objects that can be migrated to another version
pub trait Migratable<T> {
    fn migrate(&self) -> Result<T, MigrationError>;
}

// Implement Migratable for ProtocolObjectV1 to ProtocolObjectV2
impl Migratable<ProtocolObjectV2> for ProtocolObjectV1 {
    fn migrate(&self) -> Result<ProtocolObjectV2, MigrationError> {
        // Write actual migration logic
        Ok(...)
    }
}

// "Seal" that stops external code from implementing the ImmutableProtocolObject trait
trait ImmutableProtocolObjectSealed {}

// Trait for objects that shouldn't be migrated
pub trait ImmutableProtocolObject: ImmutableProtocolObjectSealed {}

// Implement the "seal" for ProtocolObjectV1
impl ImmutableProtocolObjectSealed for ProtocolObjectV1 {}

// We also should implement ImmutableProtocolObject for ProtocolObjectV1
impl ImmutableProtocolObject for ProtocolObjectV1 {}

With this code, rules about backward compatibility would be checked and enforced by a compiler. If it looks interesting, I can create a stand-alone PoC at some point.

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.

3 participants