OutCast3k / coinbin

Javascript Bitcoin Wallet. Supports Multisig, Stealth, HD, SegWit, Bech32, Time Locked Addresses, RBF and more!
https://coinb.in/
MIT License
907 stars 622 forks source link

support CHECKLOCKTIMEVERIFY #35

Open btcdrak opened 8 years ago

btcdrak commented 8 years ago

Would you consider making a the ability to create some P2SH redeem scripts using CHECKLOCKTIMEVERIFY (NOP2)

I was thinking two variants:

1 Take the standard multisig generator and turn into an IF clause that requires multisig conditions until the locktime expires, then a single address can sign. Something like

    IF
        <now + 3 months> CHECKLOCKTIMEVERIFY DROP
        <Lenny's pubkey> CHECKSIGVERIFY
        1
    ELSE
        2
    ENDIF
    <Alice's pubkey> <Bob's pubkey> 2 CHECKMULTISIG

2 Simple HODL,

`<expiry time> CHECKLOCKTIMEVERIFY DROP DUP HASH160 <pubKeyHash> EQUALVERIFY CHECKSIG`

3 Trustless payment HODL

    IF
        HASH160 <Hash160(encryption key)> EQUALVERIFY
        <publisher pubkey> CHECKSIG
    ELSE
        <expiry time> CHECKLOCKTIMEVERIFY DROP
        <buyer pubkey> CHECKSIG
    ENDIF
chris-belcher commented 8 years ago

Another option may be to allow creating and spending from any arbitrary redeem script. It would be harder to use but more extensible.

dabura667 commented 8 years ago

<expiry time> CHECKLOCKTIMEVERIFY DROP DUP HASH160 <pubKeyHash> EQUALVERIFY CHECKSIG

You would be better off just using the P2PK instead of P2PKH, as it would save you 23 bytes per input... and the redeemscript is hidden anyways (so the pubkey protection is maintained)

chris-belcher commented 8 years ago

@dabura667 I thought p2sh was the only practical way to use this?

dabura667 commented 8 years ago

@chris-belcher yes. I am speaking about the redeemscript

chris-belcher commented 8 years ago

I see, you mean check against pubKey instead of pubKeyHash

The bip65 document checks against pubKeyHash https://github.com/bitcoin/bips/blob/master/bip-0065.mediawiki#freezing-funds

However It seems petertodd checked against pubKey in the example code https://github.com/petertodd/checklocktimeverify-demos/blob/master/hodl.py#L41-L43 This was written later and is code that can actually be run, so IMO we should do it that way.

dabura667 commented 8 years ago

This was written later and is code that can actually be run, so IMO we should do it that way.

Or rather, using the Hash160 makes no sense at all, as the reason behind using P2PKH to begin with is to hide the pubkey behind a hash and not publish it to the blockchain until the utxo is used.

This goal is already accomplished by P2SH, so there's no need for the Hash160 of the pubkey in there, it's just unnecessary data.

weex commented 8 years ago

So we've got some progress in the following bounty but are having a hard time getting a spend from the locked address to be relayed. Here's the raw transaction: http://pastebin.com/ssJnNanr and I'm getting the following message from Bitcoin Core 0.11.2. {"code":-26,"message":"64: non-mandatory-script-verify-flag (No error)"} Any ideas?

chris-belcher commented 8 years ago

@weex I don't know but I'd recommend you use testnet for trying out stuff like this. Testnet coins are easy to get hold of, I have some if you need. You could try looking in debug.log, but unfortunately the script error messages are never very good.

It's only 0.2 mbtc locked up in the worst case. (But maybe with time that will become a lot)

edit: for later reminder, weex has locked up 0.0003 BTC into the address 36eagf2eUELSCy7i93hZhGJXgZ3hHdeKUJ until Wed Apr 30 2042 07:14:30 UTC.

dabura667 commented 8 years ago

@weex You're being rejected because the nLocktime is set prior to the time pushed on the stack.

CLTV failed so the Transaction is invalid.

I looked at your raw tx and saw you have the coins locked until Wed Apr 30 2042 07:14:30 UTC.

When endianness is reversed, the date is yesterday, so I'm assuming you didn't mean to lock them for 26 years.

This is why we have testnet.

Sorry for your loss.

dabura667 commented 8 years ago

Or rather, umm, temporary loss?

weex commented 8 years ago

I'll deal. Or rather the programmer and myself will both have to deal as I've made another test for a similarly small amount of Bitcoin though probably locked for another year. If we fix the endianness and post an unsigned redeem script, will someone be able to verify the nlocktime?

dabura667 commented 8 years ago

Yes, I have tested transactions already, and your transaction met all the criteria. Just the byte order was incorrect on the redeemscript. Your nLockTime byte order was correct, but I'm guessing you left the nLockTime calculation to some library.

dabura667 commented 8 years ago

abcdef34 would be 34efcdab and vice versa

OutCast3k commented 8 years ago

Sorry for the late reply here, I have had so many issues at work, then my computer at home failed on me, then Christmas!

I'm just going over this suggestion (and a couple of others) now and I will see if we can get it implemented asap.

thelink2012 commented 8 years ago

The nLockTime is wrong in the script or is in the tx field itself?

In fact, I was a bit worried about the endianess in the script as seen in a comment here, I really didn't know how to treat endianess there. Also see this, wasn't too sure either how to convert from integer to byte vector directly, so I converted to a string first and then did hexToBytes, may that be a issue?

By the way, no way one can use coinbin (or other similar service) to test through testnet? As running Core and similars manyn't be quite hmm attractive because of the blockchain size.

chris-belcher commented 8 years ago

According to this comment, you can use testnet: https://github.com/OutCast3k/coinbin/issues/2#issuecomment-114664309

dabura667 commented 8 years ago

@thelink2012 All integers in Bitcoin are encoded as little endian.

your code is not safe because you would read the four byte transaction version 0x01000000 as 16777216 instead of 1...

Considering bitcoin transactions are still on version 1... yeah... over 16 million is a large version.

Any integer pushed on the stack in Script must also be little endian.

0001 0002 OP_ADD is not 3, but is actually 768, because you're adding 256 and 512, not 1 and 2.

thelink2012 commented 8 years ago

I just tested by changing the byteorder of the locktime in a quick 'n dirty way, and that indeed worked.

When encoding I did

var lbts = Crypto.util.hexToBytes(locktime.toString(16));
s.writeBytes([lbts[3], lbts[2], lbts[1], lbts[0]]);

When decoding I did

r.locktime = parseInt(Crypto.util.bytesToHex([s.chunks[0][3], s.chunks[0][2], s.chunks[0][1], s.chunks[0][0]]), 16);

Tx with bad locktime: https://live.blockcypher.com/btc-testnet/tx/88d2ac101fda281223fcd309e30c2eded651fced3f091372502280040e0047e2/ Tx with correct locktime (using code above): https://live.blockcypher.com/btc-testnet/tx/9d165f6e41bacf0fef83007cd597f36ca0892d750d63c621ce238f70df5d379b/

Just need to do it in the way it's supposed to be done in this code base, what would it be @OutCast3k ? coinjs.numToBytes and coinjs.bytesToNum? Using Crypto.util.endian(n) to swap endianess (as Crypto.util seems to make big endian byte vectors)? Manual bitshifting?

OutCast3k commented 8 years ago

@thelink2012 Nice work.

I've not tested the code below its just based on what I've read, but what about this:

Encoding:

s.writeBytes(Crypto.util.hexToBytes(locktime.toString(16)).reverse());

Decoding:

r.locktime = parseInt(Crypto.util.bytesToHex(s.chunks[0].reverse()), 16);

also, I was thinking perhaps rename 'r.locktime' to something like 'r.checklocktimeverify' just to clearly distinguish between this value and nlocktime a bit more.

OutCast3k commented 8 years ago

@btcdrak whilst I'm going to accept thelink2012's merge for a "simple hodl address" because its useful in its own way. I like your original suggestions too, and just wanted to add that I think its something I'll work on shortly if no one else has already.

btcdrak commented 8 years ago

@OutCast3k :+1:

Anything that helps people make use of OP_HODL scripts it a good thing!

OutCast3k commented 8 years ago

@thelink2012 your work is live, I made a few tiny changes to the interface though.

https://coinb.in/#newTimeLocked

thelink2012 commented 8 years ago

@OutCast3k Absolutely, looks good.

weex commented 8 years ago

Great work @thelink2012 and @OutCast3k. Let the OP_CHECKLOCKTIMEVERIFYING begin!

dabura667 commented 8 years ago

Idea:

Even though locktime supports granularity down to the second, variations in local clocks for nodes as well as bip113 causing roughly an hour delay compared to real world time, I propose we only allow granularity down to the hour.

Cutting down the granularity to an hour makes it trivial (even in a browser with javascript on a slow computer) to brute force dates in case the user forgets the redeemscript.

Maybe this should be another BIP

zilveer commented 7 years ago

Hello everyone, Does this mean that CHECKLOCKTIMEVERIFYING is working ? If so where in the GUI can I find it ?

Thx for your help.

chris-belcher commented 7 years ago

@zilveer on coinb.in you can create a time locked address here https://coinb.in/#newTimeLocked

zilveer commented 7 years ago

@chris-belcher Is that the same as CHECKLOCKTIMEVERIFY ?

Thx for your help .

btcdrak commented 7 years ago

It would be wonderful to have a tickbox on https://coinb.in/#newTimeLocked to use OP_CHECKSEQUENCEVERIFY vs OP_CHECKLOCKTIMEVERIFY @OutCast3k