Skip to content
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

BIP 127: Simple Proof-of-Reserves Transactions #756

Merged
merged 1 commit into from Apr 4, 2019

Conversation

Projects
None yet
6 participants
@stevenroose
Copy link
Contributor

stevenroose commented Jan 29, 2019

(submitted to bitcoin-dev, awaiting approval)

@luke-jr

This comment has been minimized.

Copy link
Member

luke-jr commented Feb 15, 2019

This needs a backward compatibility section (probably simply saying it's n/a).

Also, @achow101 needs to ACK the modifications to BIP 174.

@achow101

This comment has been minimized.

Copy link
Member

achow101 commented Feb 16, 2019

You should add the new type to BIP 174's Appendix A. That should also include a link to the location in the new BIP descriibing it.

@stevenroose stevenroose force-pushed the stevenroose:por branch 2 times, most recently from 35b85c0 to 1df16a9 Feb 16, 2019

@stevenroose

This comment has been minimized.

Copy link
Contributor Author

stevenroose commented Feb 16, 2019

@achow101 I added the field to that table. I also mention the BIP in the explanation, but it's currently set to TBD until this BIP gets a number assigned, then I can update both mentions of TBD to become a hyperlink.

@luke-jr added compatibility section.

@luke-jr luke-jr changed the title [proposal] Simple Proof-of-Reserves Transactions BIP 127: Simple Proof-of-Reserves Transactions Feb 16, 2019

@luke-jr

This comment has been minimized.

Copy link
Member

luke-jr commented Feb 16, 2019

Assigned BIP 127

@stevenroose

This comment has been minimized.

Copy link
Contributor Author

stevenroose commented Feb 17, 2019

I just realized it might be useful to have a way to deal with funds that are locked in a CLTV in the future. Are their opinions about this?

I'm thinking of two approaches:

  1. have a global "locktime" variable for the entire proof, which proves that "the prover would be able to spend C coins at time L"
  2. allow specifying a locktime for some UTXOs which allows for more fine-grained locktime limitation specification, but might make the "proof statement" more complex. Something like "the prover has ownership of C1 coins now and C2 coins a time L1, ...". For UX purposes the prover can decide to use the same locktime for all it's CLTV UTXOs even though the locktime is further away for some of the UTXOs. Just to simplify the statement.

Both techniques would be quite straightforward to implement for verification.

@stevenroose stevenroose force-pushed the stevenroose:por branch from 1df16a9 to ce0dfc8 Feb 17, 2019

@stevenroose

This comment has been minimized.

Copy link
Contributor Author

stevenroose commented Feb 17, 2019

@luke-jr I updated the files to use the assigned number. Up to you if you merge it like this and allow me to update once I figured out a good way to deal with CLTV or if you want to wait for that in order to merge this.

@stevenroose stevenroose force-pushed the stevenroose:por branch from ce0dfc8 to d216d7d Feb 17, 2019

@luke-jr

This comment has been minimized.

Copy link
Member

luke-jr commented Feb 18, 2019

"the prover would be able to spend C coins at time L"

This can't be proven. The TXOs might get spent before time L.

@luke-jr

This comment has been minimized.

Copy link
Member

luke-jr commented Feb 18, 2019

First Comments-URI must be exactly "https://github.com/bitcoin/bips/wiki/Comments:BIP-0127" in bip-0127.mediawiki
@achow101

This comment has been minimized.

Copy link
Member

achow101 commented Feb 18, 2019

ACK BIP174 changes

@stevenroose stevenroose force-pushed the stevenroose:por branch 2 times, most recently from 9c481e8 to c1ef9e4 Feb 21, 2019

@stevenroose

This comment has been minimized.

Copy link
Contributor Author

stevenroose commented Feb 21, 2019

@luke-jr There is an error in the Travis that I don't understand. Seems to be related to the git history. I tried rebasing from latest master but that didn't work..

@stevenroose

This comment has been minimized.

Copy link
Contributor Author

stevenroose commented Mar 18, 2019

@luke-jr ping :)

@luke-jr

This comment has been minimized.

Copy link
Member

luke-jr commented Mar 18, 2019

It's saying you need to update the README

Show resolved Hide resolved bip-0127.mediawiki Outdated
Show resolved Hide resolved bip-0127.mediawiki Outdated

@stevenroose stevenroose force-pushed the stevenroose:por branch 2 times, most recently from 46fe591 to 7276e9a Apr 3, 2019

@stevenroose

This comment has been minimized.

Copy link
Contributor Author

stevenroose commented Apr 3, 2019

@luke-jr fixed the README

I don't currently have much time to work on this, but I think it's quite final apart from the CLTV issue. So can go in like this and I can amend with a fix for that if needed.

@rex4539

rex4539 approved these changes Apr 3, 2019

@luke-jr luke-jr merged commit e65e3fa into bitcoin:master Apr 4, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.