Skip to content
This repository has been archived by the owner on Jul 11, 2019. It is now read-only.

Separate sign and verify from swarm.js #129

Merged
merged 3 commits into from
May 20, 2015
Merged

Separate sign and verify from swarm.js #129

merged 3 commits into from
May 20, 2015

Conversation

Flet
Copy link
Member

@Flet Flet commented May 19, 2015

This PR moves the sign and verify logic out of swarm.js and into lib/signature.js.

There are a couple reasons for this:

  1. The current sign/verify logic is dependent on the filesystem and will not work in a browser. This separation will allow for an alternative way to sign/verify messages in-browser.
  2. There is a need to push swarm.js into its own module so its reusable by peerbot/friends-irc/other bots. Abstracting signature will help to keep this future module focused. Also bots probably don't need to sign/verify.

Please yell at me if this is outrageous. 😁

@Flet
Copy link
Member Author

Flet commented May 19, 2015

Hows it work?

Two new functions were added to swarm.js:

swarm.verify(fn)

fn should be a function that takes the arguments username, signature, callback that can be called to verify if the signature belongs to username. the callback is called with err, valid, with valid indicating a true if the signature could be verified.

swarm.sign(fn)

fn should be a function that takes the arguments data, callback and produces a signature for the current user with the specified data. the callback is called with the args err, signature.

signature.js is just a wrapper around ghsign which produces the required functions.

@Flet Flet mentioned this pull request May 19, 2015
4 tasks
@Flet
Copy link
Member Author

Flet commented May 19, 2015

Hmm, can someone check if the @ngoldman merge bot crashed and reboot it? 😎

emitter.verify = function (fn) {
verify = fn
}

Copy link
Member

Choose a reason for hiding this comment

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

@Flet what these be for?

Copy link
Member Author

Choose a reason for hiding this comment

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

These are to pass in functions to sign and verify messages.

swarm.js is running these because the signed/verified data is actualy the protobuf-encoded message itself, which is not readily accessible outside the module.

More info in my comment: #129 (comment)

I do agree that the variable name emitter is a little confusing. If it were named swarm it would be a bit more intuitive I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

also thank you for taking the time to review! ❤️

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes I see from #129 (comment). Might be good just to add a comment or two for now so it's clearer for future contributors since at face value they are identical and their purpose is not entirely obvious.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed! The ultimate goal is to move swarm into a nice, well docmented module :). I'll add some code comments so this information is not lost with time.

@Flet
Copy link
Member Author

Flet commented May 20, 2015

Comments for the comment god!

@ungoldman
Copy link
Member

MERGES FOR THE MERGE KING

ungoldman added a commit that referenced this pull request May 20, 2015
Separate sign and verify from swarm.js
@ungoldman ungoldman merged commit 240a68f into master May 20, 2015
@ungoldman ungoldman deleted the signature branch May 20, 2015 21:33
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.

2 participants