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

use ssb-private module #417

Closed
wants to merge 27 commits into from
Closed

use ssb-private module #417

wants to merge 27 commits into from

Conversation

ahdinosaur
Copy link
Contributor

here you go @mixmix and @mmckegg, please test if this actually works. 😺

@mixmix
Copy link
Member

mixmix commented Jun 15, 2017

awesome, this works in like a treat in patchbay.
Recommend merge publish

@mixmix mixmix changed the title [wip] use ssb-private module use ssb-private module Jun 22, 2017
@mixmix
Copy link
Member

mixmix commented Jun 22, 2017

@dominictarr how do you feel about this in master

@mmckegg in terms of how we were assessing things like night ("is this common enough that it should be a stable inclusiion?") how do you feel about this PR?

@mixmix
Copy link
Member

mixmix commented Jun 22, 2017

Basically patchbay has to use this branch at the moment. I just realised that's probably why JessyKate was eating a bunch of shit the other day 😿

@dominictarr
Copy link
Contributor

what is the advantage of having this as a separate plugin - could this just be part of the other query index?

@ahdinosaur
Copy link
Contributor Author

@dominictarr what's the benefit of having this as part of the query index? it seems we'd be mushing concerns together, as the boxing and unboxing private messages into an index based on your key pair seems separate from queries. although we did add some private functionality to the query index, i feel like we want to eventually remove that since we should have a better way to handle multiple key pairs (switching identities without breaking your indexes).

@mmckegg
Copy link
Contributor

mmckegg commented Jun 27, 2017

Yeah, it probably could be handled by the query module. And the query module probably should allow querying of private messages (like we do in ssb-backlinks).

But here are the reasons I didn't go that route:

  • We would have to have a special index in ssb-query for private: true anyway, so no space savings
  • it means that ssb-query must be reindexed (and also again every time the user changes their identity)
  • It would mean coordinating with other people to get this important feature into patchwork (to be fair, I still screwed that up by making it a replacement for scuttlebot/plugins/private instead of being a separately installable plugin).
  • I'm not actually using ssb-query in patchwork any more (replaced with the more specialized indexes like backlinks, about)

So I'm definitely open to this, but we should discuss the points above to see if it is worth it.

@dominictarr
Copy link
Contributor

@mmckegg this depends on what you are thinking about how multiple identities work? my thoughts are that your sbot knows all your identities, and it indexes all of them. This is how private groups are gonna need to work anyway.

Also, if query, backlinks, etc, all work also with private messages, then every feature that uses that can also be private. We can have a private chess game, or I can organize a private event..., etc. sbot.links works like that.

@mmckegg
Copy link
Contributor

mmckegg commented Jun 27, 2017

Here's how I would like this all to work (in a perfect world.. or maybe that's going too far):

Unboxing happens before index/reduce (can handle multiple identities, groupbox, etc here). Indexes work just like they used to, except now they have all the private messages too. The private messages get an extra attribute {value: {private: true}} so that we can filter or index against. If a user adds more identities, the indexes can probably just pick up where they left off, no need to regenerate. If a user swaps out their master identity, we might want to regenerate though.

In the case above, we would no longer need a special private index. But it's still a very useful default feed to have (probably as a query).

@arj03
Copy link
Member

arj03 commented Sep 10, 2017

Trying this branch I'm getting the following error:

    acc[data.value.author] = {id: data.key, sequence: data.value.sequence, ts: data.value.timestamp}
                  ^

TypeError: Cannot read property 'author' of undefined
    at /home/chrx/dev/scuttlebot/node_modules/secure-scuttlebutt/indexes/last.js:21:19
    at /home/chrx/dev/scuttlebot/node_modules/flumeview-reduce/inject.js:141:41
    at /home/chrx/dev/scuttlebot/node_modules/pull-stream/sinks/drain.js:24:37
    at /home/chrx/dev/scuttlebot/node_modules/pull-stream/throughs/map.js:19:9
    at /home/chrx/dev/scuttlebot/node_modules/secure-scuttlebutt/node_modules/pull-cursor/skip.js:14:16
    at /home/chrx/dev/scuttlebot/node_modules/secure-scuttlebutt/node_modules/pull-cursor/cursor.js:32:13
    at /home/chrx/dev/scuttlebot/node_modules/secure-scuttlebutt/node_modules/flumelog-offset/frame/recoverable.js:48:11
    at /home/chrx/dev/scuttlebot/node_modules/secure-scuttlebutt/node_modules/aligned-block-file/blocks.js:72:11
    at get (/home/chrx/dev/scuttlebot/node_modules/secure-scuttlebutt/node_modules/aligned-block-file/blocks.js:28:7)
    at next (/home/chrx/dev/scuttlebot/node_modules/secure-scuttlebutt/node_modules/aligned-block-file/blocks.js:51:7)

@arj03
Copy link
Member

arj03 commented Sep 11, 2017

I printed the data variable in that place and it appears that it is encrypted when passed to the index.

@mixmix
Copy link
Member

mixmix commented Sep 11, 2017 via email

@arj03
Copy link
Member

arj03 commented Sep 11, 2017

With this PR (ssbc/ssb-private#2) I'm getting a little bit further, but it seems like ssb-private makes a lot of messages null. So I'm hitting this and tons of other places:

      if(msg.sync) return
            ^

TypeError: Cannot read property 'sync' of undefined
    at /home/chrx/dev/scuttlebot/plugins/gossip/init.js:20:13

This is just running scuttlebot, nothing connected.

@dominictarr
Copy link
Contributor

ssb-private probably needs a pull.filter(Boolean) after it attempts decryption, to filter out the messages which couldn't be decrypted

@arj03
Copy link
Member

arj03 commented Sep 12, 2017

Oh ho ho. It appears my local scuttlebot checkout was fucked up. After clearing everything and installing things again this branch is now working for me!

@dominictarr
Copy link
Contributor

Hey, so I was just about to merge this (I gave in, and used it to in patchless...) and it was working in the version I had cloned and symlinked (probably an npm install from a while back)

but then when I tried doing installing it via npm... it just didn't work, some weird error. so I looked at the code... to my disapointment... I see my own code copied into the module!!! when I eventually figure out what the problem was... I change _ops.value = false to true and now it works in my code, does this break patchwork? I don't know, but if this was a pull request to flumeview-query well, that actually has tests. If you made a PR and I merged it that means I'd accept the responsibility for maintaining it.

I will not merge this pr until ./lib/flumeview-links-raw is instead a PR to flumeview-query.

@dominictarr dominictarr closed this Mar 5, 2018
@dominictarr
Copy link
Contributor

Oh, sorry I see there were actually some issues that hadn't responded to since last time I got upset about this.

@dominictarr
Copy link
Contributor

but let the record show that if you copy my code, change it, and don't tell me about that... I no longer accept any responsibility to maintain that code. In fact, I feel that is actively counter productive - it is not worthwhile except in the absolute shortest of short terms. It's especially infurating when we are basically working on the same project and I'm just over here.

@mmckegg
Copy link
Contributor

mmckegg commented Mar 6, 2018

@dominictarr

I change _ops.value = false to true and now it works in my code, does this break patchwork?

Are you talking about the code here: https://github.com/ssbc/ssb-private/blob/master/lib/flumeview-links-raw.js#L83

That code is already opts.values = true which works fine in patchwork.

I will not merge this pr until ./lib/flumeview-links-raw is instead a PR to flumeview-query.

Suit yourself. Getting this PR merged doesn't affect Patchwork. If @mixmix, @ahdinosaur or someone else wants this in sbot, they're welcome to figure out how to make it work!

Unsubscribing....

@dominictarr
Copy link
Contributor

in scuttlebot@11 I have removed the private plugin so it should no longer conflict with patchwork.

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.

None yet

5 participants