Transaction full cleanup

[mattgstevens]

this is from transaction-cleanup, changes are in blockchainAPI:

• correct use of includes
• no JSON.parse on return of blockchain.buildTransaction

[fromheten]

After a long period of this being neglected I'm trying to restore from a dump and make a transaction. Here are my notes from there. I'm doing this on current branch, head is ab716b2.

https://gist.github.com/fromheten/38546ded9df089c65618d7519f61f680

Maybe it's just easier to throw away the things we wrote about transactions and re-do it.

[mattgstevens]

Need to confirm, but could it be that there are some record inconsistencies? There are updates to Invoice and Content lookups / the way redis saves these. The production snapshot has old data records as it was running older api version at the times you were demo-ing.

Will take a look at this at the end of working day today.

[fromheten]

My mistake was having appendonly flag on in redis.conf, which will ignore the dump.rdb file. Also my dump is in the old format, where invoices are on the first level.

Lemme know if you're free for a skype some day and maybe we can pair on this (screen sharing and whatnot). Would be cool :)

[fromheten]

Martins test log

• [x] Create a new invoice
• [x] Log the WIF and other invoice data
• [x] Pay the invoice
• [x] Run the payout script when in this branch
• [x] Report back

Invoice data

My newly created invoice

{ contentId: 'momona-demo-video',
  address: '1EeTb8ToQDn3FVLT63oZUrDWcsqknCxG1g',
  privateKey: 'Kz2z3FgBhJ8RAh1o4EuVH534eLtdZdsoxiiGotTphk3hB6mj4JM1',
  createdAt: 1468983639134 }

Screenshot of http://localhost:3000/0/content/momona-demo-video?address=1EeTb8ToQDn3FVLT63oZUrDWcsqknCxG1g

Payin transaction

Blockchain.info link

Payout log

Current git head

~/Repos/momona/bitcoin-remunerate-api$git log -n1
commit ab716b27eb5455b1f056aeff088841a68fea284e
Author: mattgstevens <mattgstevens@gmail.com>
Date:   Sat May 21 17:35:01 2016 +0200

    fix: blockchainApi.broadcastTransaction does not return JSON

Successful transactions creation and payout!

~/Repos/momona/bitcoin-remunerate-api$npm run repl ./scripts/payout-to-contentId.js momona-demo-video 
> bitcoin-remunerate-api@1.0.0 repl /Users/martin/Repos/momona/bitcoin-remunerate-api
> NODE_PATH=./src node "./scripts/payout-to-contentId.js" "momona-demo-video"

▀ ╢░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░╟
payout momona-demo-video
transaction: Broadcasting 010000000173e63b59f0dc85f971ce02c9852aa585ac88a8a6d3b7e8c1e5d352b2ca553af4000000006a47304402207c0bf7146e8346683d3ffa6398cbfe50b5005b30974e38c799021ec32b641bce0220454ff97daed7056919014dd1bb63fb1b61fd05f1bff0324e984c563e1d080243012103ee338010d93c842a7d1c935df601768ddcf0b9a7389e5768144e5e775b5dfc8cffffffff02b43b0000000000001976a914610270d1f04ca037dffc29bbb2c7f0d576c6b04488ace7050000000000001976a914610270d1f04ca037dffc29bbb2c7f0d576c6b04488ac00000000
transacton: Message from blockchain.info Transaction Submitted

result Transaction Submitted

▀ ╢░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░╟

Electrum transaction log showing the payin followed by payout transaction (sorted by time, newest first)

Payout transaction on www.blockchain.info

Conclusion

I conclude that the code works. I say LGTM.

Cost of running test

• Price paid to make the payin transaction, including fees: BTC 0.00022296
• Money received after payout: BTC 0.00016795
• Difference AKA cost of doing this test: BTC 0.00005501 So the price tag for a test is 0.03 euro.

Special cases when I tested

• Lowered the minimum payout amount by one order of magnitude - has no impact on anything
• Changed the price for momona-demo-video from 1000 satoshi to 1 SEK as sending 1000 satoshi is a waste of time

My take-aways from this PR

This is the longest I've ever seen a PR be open, and it was at times not fun.

Especially having working code, and then it being changed and we lost track of at what point it was working.

To solve that I propose these guidelines:

• Only one person commits to one branch. There's the saying "the more cooks, the worse the soup" but I think it really boils down to (lol, pun indented (that one too)) git branches being shared mutable state. We programmers are independent actors, changing this one thing. The solution is to propose changes in pull requests. You can request to merge one branch into any other. Then updates are atomic and explicit.
• To merge before doing refactorings and style improvements. Better to open a new pull request later with style improvements, if one finds them important.
• That everyone involved sets up an environment where they can test. Since we are dealing in Bitcoin, that means having a wallet on ones computer loaded with like €5 just for testing things out.
• We write a script to automate the process I did above. It's not hard, but it's kind of boring to go thru the steps over and over again when much of it can be automated in a script. Since tests cost €0.03 it should not run on every file save, but running it once before merging some PR is such a low cost it's basically nothing.

In short, preferring to merge earlier than later and keeping a 1:1 relationship between branches and programmers. Super psyched to merge this and then get on to writing documentation and making a Show HN.

[fromheten]

Immediate next steps

• [ ] @mattgstevens, you are very welcome to reproduce this test if you wish.
• [ ] Merge it if you agree that it's satisfactory
• [ ] I look forward to hearing your comments and discussion, this PR thread would be a good place. Let's merge before we discuss, so that while the discussion runs here we can get other shit done.

[fromheten]

For future reference: this PR is a continuation of https://github.com/EndlessVanguard/bitcoin-remunerate-api/pull/35 from April 24 2016.

[mattgstevens]

Merged!

100% on point in take-aways section. Long time in PR + multiple people making commits + refactoring on a feature branch make Matt and Martin sad and Momona development slow.

Opened an issue #47 to automate the testing of payments / payouts so we know things are all good in for blockchain side effects.