-
Notifications
You must be signed in to change notification settings - Fork 9
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
letter shop tutorial #1
Conversation
I just picked the Merlot theme in gh-pages config, we can change it to something else. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I really like where this is going. I think there are a couple things that could be changed to make the documentation cleaner or the setup steps faster, but the idea is funny and it's a good way to get started.
letter-shop.md
Outdated
Save the following JavaScript as `shop.js`: | ||
|
||
```js | ||
const http = require('http') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about using something like koa instead? I think it's pretty uncommon in the Node ecosystem to use the http module directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm, interesting argument. I don't think I've ever met a NodeJS developer who was unfamiliar with the http module though, have you? And I definitely know there are people the other way around, for instant backend developers (including myself) would be more familiar with the http module than with koa. Also, in the second tutorial I need to flush the headers, so I think it wouldn't work there.
letter-shop.md
Outdated
} else { | ||
const secret = crypto.randomBytes(32) | ||
const fulfillment = base64(secret) | ||
const condition = base64(crypto.createHash('sha256').update(secret).digest()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: put some of the function calls on different lines or make the code area wider so the whole line fits without scrolling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, the code area is annoyingly narrow, but then also having to use linebreaks everywhere hurts the readability. I've gone back and forth on that. Maybe I can make the code area wider somehow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed it to Modernist now, which gives 23 more chars width (up from 61 to 84). We could get another 8 chars by switching to Cayman, and that theme may fit better in the Interledger house style, but probably looks less fancy for tutorials.
letter-shop.md
Outdated
|
||
## Step 1: | ||
|
||
Save the following JavaScript as `shop.js`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about making a repo that contains all of the example code that you just clone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, while I was writing #2 I ended up adding the code in this repo, I'll add this tutorial's files to this PR.
letter-shop.md
Outdated
|
||
plugin.connect().then(function () { | ||
plugin.on('incoming_prepare', function (transfer) { | ||
plugin.fulfillCondition(transfer.id, fulfillments[transfer.executionCondition]).catch(function () {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an insecure way to handle fulfilling payments (because you're not checking the amount). We should not encourage people to do this themselves because they are likely to make mistakes like this and lose money. The example should use the ilp
module instead. If neither PSK nor IPR is suitable for this we should discuss why they aren't, come up with an alternative, and include it in that module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check the amount (the second tutorial does check the transfer amount against the IPP amount). Checking the expiry is already done by the ledger. I don't think we need to include a module for that?
The payment request here is a human-readable phrase. Further on, the proxy actually checks if the first word on the page is 'Please', and only pays if it is. :) I'm starting with this simplistic flow on purpose; using IPR would just hide the under-the-hood semantics from sight.
I agree about setting a price per letter though, and then checking that. It's currently a 'pay what you want' shop and that's just silly. :)
letter-shop.md
Outdated
npm install michielbdejong/ilp-plugin-xrp-escrow#3fadeb4 | ||
node shop.js | ||
``` | ||
In the code, you see that an 'ilp-plugin-xrp-escrow' Plugin is being configured with secret, account, server, and prefix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are very dense paragraphs. It would be good to break them up into much shorter sections and maybe put some headings above them to help when the reader is skimming
letter-shop.md
Outdated
* the Ledger Plugin Interface (LPI) | ||
* how to build a paid web app using Interledger! | ||
|
||
## Step 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe make this heading more descriptive like Step 1: Creating the Letter Shop
letter-shop.md
Outdated
In the case of Interledger, the fulfillment is always a 32-byte string, and the condition is the sha256 hash of that string. | ||
SHA256 is a one-way hash function, so if you know `fulfillment`, then it's easy and quick to calculate `condition = sha256(fulfillment)`, but if you only know the condition, if you only have a million years or less, it's near-impossible to find (i.e., guess) a fulfillment for which `condition = sha256(fulfillment)` would hold. And in practice, we tend to use rollback timeouts that are of course much shorter! :) Because the transfer is *locked* until the recipient produces the correct fulfillment, the condition of the transfer is a *hash* of its fulfillment, and the transfer will *time* out after a while if the recipient doesn't produce the fulfillment to claim the funds, we call this type of conditional transfer a *Hash Time Lock Contract (HTLC)*. We call this a contract between sender an recipient, that is enforced by the ledger. If the sender and the recipient would exchange the condition and the fulfillment directly, that would be an off-ledger transaction, and instead of a Hash Time Lock Contract, we would use the more general term Hash Time Lock Agreement, to indicate that the interaction happens without having the ledger as an arbitor. | ||
|
||
## Paying for your letter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add "Step 2:"
letter-shop.md
Outdated
The plugin used here is specific to XRP, and to the use of on-ledger escrow. Escrow for XRP is described [here](https://ripple.com/build/rippleapi/#transaction-types). Escrow transfers differ from normal transfer in that the recipient doesn't automatically receive the amount of the transfer in their account; the need to produce something in order to claim the funds. During the time between the sender's action of preparing the transfer (creating the escrow), and the time the recipient produces the fulfillment for the transfer's condition, the money is on hold on the ledger. If the recipient doesn't produce the fulfillment in time, the transaction is canceled, and the money goes back into the sender's account. | ||
|
||
In the case of Interledger, the fulfillment is always a 32-byte string, and the condition is the sha256 hash of that string. | ||
SHA256 is a one-way hash function, so if you know `fulfillment`, then it's easy and quick to calculate `condition = sha256(fulfillment)`, but if you only know the condition, if you only have a million years or less, it's near-impossible to find (i.e., guess) a fulfillment for which `condition = sha256(fulfillment)` would hold. And in practice, we tend to use rollback timeouts that are of course much shorter! :) Because the transfer is *locked* until the recipient produces the correct fulfillment, the condition of the transfer is a *hash* of its fulfillment, and the transfer will *time* out after a while if the recipient doesn't produce the fulfillment to claim the funds, we call this type of conditional transfer a *Hash Time Lock Contract (HTLC)*. We call this a contract between sender an recipient, that is enforced by the ledger. If the sender and the recipient would exchange the condition and the fulfillment directly, that would be an off-ledger transaction, and instead of a Hash Time Lock Contract, we would use the more general term Hash Time Lock Agreement, to indicate that the interaction happens without having the ledger as an arbitor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe link to the HTLA explanation doc in case people want (a lot) more details
letter-shop.md
Outdated
sendTransfer({ | ||
to: process.argv[2], | ||
amount: '1', | ||
executionCondition: process.argv[3] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: The fact that it's pulling these options from the command line is a little bit hard to find since they're buried down here. Maybe put them up at the top in a variable so it's the first thing you see (after the imports)
letter-shop.md
Outdated
obj.from = plugin.getAccount() | ||
// to | ||
obj.ledger = plugin.getInfo().prefix | ||
// amount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, where's the amount?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All this function does is add the "standard" fields. You still need to set the "interesting" fields yourself, see for instance how it gets called here.
This is awesome! I recommend merging and starting to edit based on wider feedback from people doing the tutorial. |
Updated for IL-RFC-4, draft 6. |
@@ -0,0 +1 @@ | |||
theme: jekyll-theme-modernist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
letter-shop.md
Outdated
|
||
## Step 1: Creating the Letter Shop | ||
|
||
Save the following JavaScript as `shop.js`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should start with the instructions for cloning and npm install
ing the repo instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. An advantage of that would be that we can then discuss shorter snippets of the code and not have to paste entire files - especially where shop.js and shop2.js and shop3.js from the next tutorial have a lot in common.
letter-shop.md
Outdated
} | ||
}).listen(8000) | ||
}) | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to read this from the perspective of someone who hasn't seen our code before. It feels like this code could use more explanation of what it's doing. What do you think about either:
- Walking people through building up this example, explaining what each step is doing or
- Breaking down the code segment by segment after you show the full example or
- Adding more comments in the code about what's going on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, if we tell people to clone the repo then we have more freedom to break down the snippets into smaller ones.
letter-shop.md
Outdated
Set up a Letter Shop website on http://localhost:8000, by running: | ||
|
||
```sh | ||
npm install michielbdejong/ilp-plugin-xrp-escrow#3fadeb4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think this is necessary now that it's in the package.json
letter-shop.md
Outdated
``` | ||
|
||
### Interledger addresses | ||
In the code, you see that an 'ilp-plugin-xrp-escrow' Plugin is being configured with secret, account, server, and prefix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably need to explain that plugins are abstractions over the functionality of different ledgers before going to Interledger details
letter-shop.md
Outdated
|
||
## Step 3: Paying proxy | ||
|
||
It's of course very cumbersome to cut and paste the condition from your browser to your command-line terminal each time you need to pay for something online, and then to cut and past back the fulfillment from your terminal to your browser once you paid. Therefore, the following paying proxy is useful, which parses the shop's payment instructions, executes them, retreives the paid content, and serves it on port 8001. Save it as `proxy.js`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also don't need to save this anymore
letter-shop.md
Outdated
executionCondition: parts[13] | ||
}).then(function (transferId) { | ||
console.log('transfer sent', transferId) | ||
pendingRes[transferId] = outRes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be cleaner to add an event listener on the plugin here instead of storing the responses in that global object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one listener per transferId? and then remove it once called, using arguments.callee
, I guess?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, using a named function. ok, that works! :)
letter-shop.md
Outdated
|
||
Now run: | ||
```sh | ||
npm install node-fetch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessary
letter-shop.md
Outdated
* `incoming_prepare` event (in `shop.js`, is triggered when someone else sends you a conditional transfer) | ||
* `outgoing_fulfill` event (in `pay.js` and `proxy.js`, is triggered when someone else fulfills your conditional transfer) | ||
|
||
If you read the paragraphs above, you will have seen the following new words; see the glossary in [IL-RFC-19, draft 1](https://interledger.org/rfcs/0019-glossary/draft-1.html) as a reference if you |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it's necessary to include these terms at the bottom but if you want to have them it might be worth linking each to its definition in the glossary
"dependencies": { | ||
"ilp-packet": "^1.3.0", | ||
"ilp-plugin-payment-channel-framework": "github:interledgerjs/ilp-plugin-payment-channel-framework#bs-clp", | ||
"ilp-plugin-xrp-escrow": "github:michielbdejong/ilp-plugin-xrp-escrow#72e4178", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this using a fork of the normal plugin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mainly because of ripple-unmaintained/ilp-plugin-xrp-escrow#18
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should get merged, but it also doesn't seem necessary for the basic tutorial, no?
letter-shop.md
Outdated
* a laptop with an internet connection and NodeJS installed | ||
* basic knowledge of the command line terminal | ||
* basic knowledge of JavaScript, including Promises and Buffers | ||
* (optional) the concept of one-way hash functions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like git is now a prerequisite
letter-shop.md
Outdated
``` | ||
|
||
### Interledger plugins | ||
Since Interledger connects potentially very different ledgers together, we need an abstraction layer that hides the specifics of the ledger, but exposes the interface needed to send and receive money over that ledger. That is what Interledger plugins are for. The 'XRP escrow' plugin is a wrapper around [RippleLib](https://github.com/ripple/ripple-lib), and exposes the Ledger Plugin Interface (LPI). In the code, you see that an 'ilp-plugin-xrp-escrow' Plugin is being configured with secret, account, server, and prefix. The first three come from the [XRP Testnet Faucet](https://ripple.com/build/xrp-test-net/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should link to the Ledger Plugin Interface
pay.js
Outdated
prefix: 'test.crypto.xrp.' | ||
}) | ||
|
||
function sendTransfer (obj) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not strictly necessary but I think it would be nice to add a bunch of comments in all of the source files detailing what's going on
|
||
* the hashlock concept | ||
* the Ledger Plugin Interface (LPI) | ||
* how to build a paid web app using Interledger! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that you don't have to copy and paste the code anymore but it almost feels too quick now, like it's missing the actual code walkthrough. What do you think about the tutorial style of something like React where they show small snippets next to explanation and then also provide the full code of the example? (They also link to codepen, which is cool cause there's no need to download it to play around with it, but then again it is a frontend framework)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about the tutorial style of something like React where they show small snippets next to explanation and then also provide the full code of the example?
That's my preferred style of tutorial. Snippets inside code and then full code at the end.
The Kotlin Tutorials have a very cool way of showing code snippets that you can expand and execute: https://kotlinlang.org/docs/reference/basic-syntax.html
Not sure what library they use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to merge now. We should send this to a couple people and get more feedback from people less familiar with Interledger before we publicize it very widely.
letter-shop.md
Outdated
The plugin used here is specific to XRP, and to the use of on-ledger escrow. Escrow for XRP is described [here](https://ripple.com/build/rippleapi/#transaction-types). Escrow transfers differ from normal transfer in that the recipient doesn't automatically receive the amount of the transfer in their account; the need to produce something in order to claim the funds. During the time between the sender's action of preparing the transfer (creating the escrow), and the time the recipient produces the fulfillment for the transfer's condition, the money is on hold on the ledger. If the recipient doesn't produce the fulfillment in time, the transaction is canceled, and the money goes back into the sender's account. | ||
|
||
### Hashlocks | ||
In the case of Interledger, the fulfillment is always a 32-byte string, and the condition is the sha256 hash of that string. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This description kind of reads like you expect the reader to know what condition and fulfillment mean, but those terms are somewhat specific to Interledger
letter-shop.md
Outdated
To send an Interledger payment, call the `sendTransfer` method of any connected Interledger plugin: | ||
|
||
```js | ||
function sendTransfer (obj) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's more confusing for there to be a separate function called sendTransfer. Why not just put these parameters into the call below?
Good idea, let's see how we can find these people. We can discuss it in the community call now. |
Try it out here: https://michielbdejong.github.io/tutorials/letter-shop
Code from the scripts is also here: https://github.com/michielbdejong/letter-shop