Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

Bridges DNA definition #752

Merged
merged 19 commits into from
Dec 19, 2018
Merged

Bridges DNA definition #752

merged 19 commits into from
Dec 19, 2018

Conversation

lucksus
Copy link
Collaborator

@lucksus lucksus commented Dec 12, 2018

Update

I've applied @zippy's change request to have a enum BridgeReference and I just reused struct dna::capabilities::Capability for the BridgeReference::Capability variant.

This way we can already specify a bridge dynamically through function signatures :)

Addition to DNA spec:

"bridges": [
    {
        "presence": "required",
        "handle": "DPKI",
        "reference": {
            "dna_address": "Qmabcdef1234567890"
        }
    },
    {
        "presence": "optional",
        "handle": "Vault",
        "reference": {
            "capabilities": {
                "persona_management": {
                    "capability": {
                        "membrane": "public"
                    },
                    "functions": [
                        {
                            "name": "get_persona",
                            "inputs": [{"name": "domain", "type": "string"}],
                            "outputs": [{"name": "persona", "type": "json"}]
                        }
                    ]
                }
            }
        }
    }
]

As per discussion with @zippy and @artbrock on 13/12/2018 I've moved the bridge definitions into the zome.

@lucksus lucksus added the review label Dec 12, 2018
@lucksus lucksus requested a review from zippy December 12, 2018 23:47
@philipbeadle
Copy link
Contributor

We also need to be able to setup bridges at runtime, eg when a new private chat group is setup that has its own DHT :)

@codecov
Copy link

codecov bot commented Dec 13, 2018

Codecov Report

❗ No coverage uploaded for pull request base (develop@72021eb). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #752   +/-   ##
==========================================
  Coverage           ?   72.43%           
==========================================
  Files              ?      146           
  Lines              ?     4953           
  Branches           ?        0           
==========================================
  Hits               ?     3587           
  Misses             ?     1366           
  Partials           ?        0
Impacted Files Coverage Δ
core_types/src/dna/dna.rs 87.7% <100%> (ø)
core_types/src/dna/zome.rs 95.24% <100%> (ø)

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 72021eb...33f972b. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 13, 2018

Codecov Report

Merging #752 into normalize-dna-module will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@                   Coverage Diff                    @@
##           normalize-dna-module     #752      +/-   ##
========================================================
+ Coverage                 73.53%   73.55%   +0.03%     
========================================================
  Files                       140      140              
  Lines                      4733     4737       +4     
========================================================
+ Hits                       3480     3484       +4     
  Misses                     1253     1253
Impacted Files Coverage Δ
core_types/src/dna/dna.rs 88.06% <100%> (+0.76%) ⬆️

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 b736d5a...0751e5e. Read the comment docs.

@lucksus lucksus changed the base branch from normalize-dna-module to develop December 14, 2018 13:25
@lucksus
Copy link
Collaborator Author

lucksus commented Dec 14, 2018

@philipbeadle, we talked about your point yesterday. I see setting up these bridges as container concern. The UI that deals with several apps/DNAs/DHTs needs to have admin privileges (or rely on some signalling system) to install private-channel-DNAs. Those channel DNAs define (statically) that they can or need to bridge to some other DNA (Vault or a root chat DNA). They can do that by either locking in the DNA hash of that dependency (v1) or by defining a trait, that is set of functions they expect in their bridge app (follow-up)

Does that match with what you're thinking?

core_types/src/dna/dna.rs Outdated Show resolved Hide resolved
core_types/src/dna/dna.rs Outdated Show resolved Hide resolved
core_types/src/dna/zome.rs Outdated Show resolved Hide resolved
core_types/src/dna/zome.rs Outdated Show resolved Hide resolved
core_types/src/dna/bridges.rs Outdated Show resolved Hide resolved
@lucksus lucksus mentioned this pull request Dec 18, 2018
@lucksus
Copy link
Collaborator Author

lucksus commented Dec 18, 2018

See updated description:

I've applied @zippy's change request to have a enum BridgeReference and I just reused struct dna::capabilities::Capability for the BridgeReference::Capability variant.

This way we can already specify a bridge dynamically through function signatures :)

@lucksus
Copy link
Collaborator Author

lucksus commented Dec 18, 2018

@sphinxc0re please re-review.
@zippy, I know you have change Capabilities already in your branch. I would merge this one if you agree with the latest changes. Or we merge your's first and I fix conflicts in here. Anyway, I think we should have both merged in tomorrow and pair a bit on the following steps :)

@lucksus
Copy link
Collaborator Author

lucksus commented Dec 18, 2018

@philipbeadle, reusing the Capabilities struct we had in the DNA already I got this in quickly. Do think this covers everything you need? The root chat app could have a special capability meant for the private channels. That capability gets specified in the private channel DNA like above. The container can then match and suggest (via admin UI or yet TBD signal/request system) that these two DNAs can be bridged and change the container config accordingly if the user chooses so.

Copy link
Member

@zippy zippy left a comment

Choose a reason for hiding this comment

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

Yes. I like this. I think it will work well.

Copy link
Contributor

@sphinxc0re sphinxc0re left a comment

Choose a reason for hiding this comment

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

The Default implementation for Zome could be derived automatically, I think 🤔

core_types/src/dna/bridges.rs Show resolved Hide resolved
@lucksus lucksus merged commit 256f874 into develop Dec 19, 2018
@lucksus lucksus removed the review label Dec 19, 2018
@zippy zippy deleted the bridges-dna-definition branch July 4, 2019 19:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants