Skip to content

Fix for setupRefund bug (Issue #19)#25

Open
illuzen wants to merge 2 commits intobitpay:masterfrom
illuzen:master
Open

Fix for setupRefund bug (Issue #19)#25
illuzen wants to merge 2 commits intobitpay:masterfrom
illuzen:master

Conversation

@illuzen
Copy link

@illuzen illuzen commented Jun 4, 2015

Hey I fixed the bug was preventing the refund exchange from getting thru, cuz we're using bitcore and need payment channel code to work.

It seems to be something to do with how Transaction.change was being used, so I used Transaction.to in place of it, calculating the fees manually.

I tested it on code communicating with BitcoinJ and it successfully did the refund exchange.

Wasn't able to figure out how to do gulp lint etc. on it, as there was no lint target in the gulp file, but I don't understand gulp very well. If anybody wants to help me out with that, I'll run the unit tests and stuff.

@braydonf
Copy link
Contributor

braydonf commented Jun 4, 2015

Can you check the status of this with the latest merge?

@illuzen
Copy link
Author

illuzen commented Jun 4, 2015

It looks like the latest merge covers the issues I was addressing, but more cleanly. Next time!

@illuzen illuzen closed this Jun 4, 2015
@illuzen illuzen reopened this Jun 6, 2015
@illuzen
Copy link
Author

illuzen commented Jun 6, 2015

I take that back. There's still the issue that setupRefund expects the commitmentTx to have an output, and it doesn't. I think it should be in processFunding:

Consumer.prototype.processFunding = function(utxo) {
$.checkArgument(_.isObject(utxo), 'Can only process an array of objects or an object');
this.commitmentTx.from(utxo);
this.commitmentTx.to(this.commitmentTx.address, utxo.amount - this.commitmentTx.getFee());
};

commitmentTx.address is a P2SH 2-of-2 address with both provider's and consumer's public keys.

Reopening this to get attention on it. I'll make another pull request if it makes sense.

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.

2 participants