blockades / scuttle-dark-crystal

API for validating, building, publishing and reading Dark Crystal records
http://darkcrystal.pw
MIT License
42 stars 2 forks source link

Feature/backward compatibility #29

Closed ameba23 closed 5 years ago

ameba23 commented 5 years ago

allow for backwards compatibility with old shard messages by handling messages based on their version number

ameba23 commented 5 years ago

I've not got very far on this and i've hit a problem.

when we return shards the 'normal' way (forget about forwarding for now) they are sent not in a darkcrystal/... message but in a message built by scuttle-invite. These messages do indeed have a version number. But it is the version of scuttle-invite, not of dark crystal.

This is perhaps not a big problem, because reply messages are only return to their original author meaning we can get the version from the associated root message. Which is IMO the simplest way to get around this, and is what i propose to do.

But i wanted to mention it here in case it crops up again...

ameba23 commented 5 years ago

hmm... doing this withing ./isReply is gonna mean it needs to be an async function, less pleasant to use in a filter

m4gpi commented 5 years ago

A possible fix Peg, and it wouldn't take much, is to ditch scuttle-invite and port the codebase into scutte-dark-crystal and ssb-dark-crystal-schema. It will likely cause us less issues in the long-run I imagine.

m4gpi commented 5 years ago

To handle the different versions we could do something similar to what's going on in ssb-dark-crystal-schema.

// lib/secrets.js

module.exports = (version) => ({
  "1.0.0": require('secrets.js-grempe'),
  "2.0.0": require('./secretsWrapper')
})

So can be initialised like...

const Secrets = require ('./lib/secrets')

const secrets = Secrets(content.version)
if (!secrets) return callback(new Error('bad version')
var secret = secrets.combine(shares)
ameba23 commented 5 years ago

@KGibb8 this is a good idea ( wish i'd seen it sooner, started working on a another way of doing it )

with version 1.0.0 it's not quite a simple as requiring secrets.js-grempe - we still need to convert to hex in our wrapper, and catch errors in the validation method, so it would be a much simpler secretsWrapper...

i'll leave it like this for the moment, the next thing that needs to happen before we can properly test this is to bump the exported SCHEMA_VERSION in ssb-dark-crystal-schema to 2.0.0 (or 1.0.2 or whatever)

but i like your proposal and i think its worth doing because at some point things will start to look a mess otherwise....

ameba23 commented 5 years ago

So i've moved the forwarding stuff to a v2, as now v1 schema messages wont recombine with the original way of doing it.

if this gets reviewed and merged, tests here should pass: https://github.com/blockades/ssb-dark-crystal-schema/pull/15

mixmix commented 5 years ago

hey @ameba23 I find this really fascinating, AND I'm gong to pause doing any more review and draft up a complete design of he flows, because I think that was the goal for this week and it's a bit of a blocker for further work. Enjoying reading your work though <3

ameba23 commented 5 years ago

any blocks to merging this? i feel like theres plenty of room for improvement, and a few things have come up that need sorting - but some of them (eg refactoring recombining into a single method) dont really belong in this PR 'backward compatibility' anyway.

mixmix commented 5 years ago

I want to review this completely after we've researched the Dominic stuff. Talk to you soon.

Ps we've been doing pretty spotty all over the show reviews compared to my normal preferred process. Look forward to settling into something more regular and predictable

On Wed, 21 Nov 2018, 10:31 ameba23, notifications@github.com wrote:

any blocks to merging this? i feel like theres plenty of room for improvement, and a few things have come up that need sorting - but some of them (eg refactoring recombining into a single method) dont really belong in this PR 'backward compatibility' anyway.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/blockades/scuttle-dark-crystal/pull/29#issuecomment-440436551, or mute the thread https://github.com/notifications/unsubscribe-auth/ACitns1tpoYVr6DjG7h5QOi_U3nvwc8aks5uxHSYgaJpZM4Yl-Xi .

mixmix commented 5 years ago

merging this into dev, then doing a final review of all that work there.