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

2019 May Spec Draft proposal #143

Merged
merged 1 commit into from Dec 3, 2018

Conversation

@schancel
Copy link
Collaborator

schancel commented Oct 17, 2018

Propose a specification for May 2019 to be enforced at UNIX timestamp 1557921600.

The following changes are included without further elaboration. Additional documents
to be included later.

  • Enable the following opcodes
    • OP_MUL
    • OP_INVERT
    • OP_LSHIFT
    • OP_RSHIFT
  • Enable schnorr signatures

This commit is ment to start discussion to start discussion.

@schancel schancel force-pushed the schancel:may-spec branch 2 times, most recently from 654442c to 1122d07 Oct 17, 2018

@markblundeberg

This comment has been minimized.

Copy link
Collaborator

markblundeberg commented Oct 17, 2018

Re: New arithmetic opcodes: given some recent discussion about doing fancy math like Rabin signatures, it may be helpful to jump straight into allowing CBigNums for all arithmetic so that people don't need awkward manipulations constrained by 32 bit arithmetic. Is this a move that could be done at the same time, or is it sufficiently dangerous that it should be considered more deeply and postponed for later date?

@schancel

This comment has been minimized.

Copy link
Collaborator Author

schancel commented Oct 17, 2018

Re: New arithmetic opcodes: given some recent discussion about doing fancy math like Rabin signatures, it may be helpful to jump straight into allowing CBigNums for all arithmetic so that people don't need awkward manipulations constrained by 32 bit arithmetic. Is this a move that could be done at the same time, or is it sufficiently dangerous that it should be considered more deeply and postponed for later date?

Hard to say. BigNums introduce quite a bit of overhead. The spec for them needs to be very detailed, as there will need to be limits on the maximum size supported.

@floreslorca

This comment has been minimized.

Copy link

floreslorca commented Oct 17, 2018

Any other schnorr sig related improvements? how is that going to be introduced?

@markblundeberg

This comment has been minimized.

Copy link
Collaborator

markblundeberg commented Oct 17, 2018

Before I forget -- I would like to propose to soften the 100 byte tx limit that will have been activated. Change to "transactions may not be exactly 64 bytes" which should suffice to prevent the merkle tree ambiguity.

Some notes and rationale:

  • The shortest economic tx (1 input P2PK -> 1 output P2SH) would be at least ~150 bytes. The 100 byte limit is not going to affect any economic activity except for some short coinbases.
  • Since August 1 2017 there have been a total of 16 BCH txes under 100 bytes, most around the 85-99 byte range. Four of them were coinbases, the rest were some strange script tests.
  • The shortest possible tx is 60 bytes: one input with empty scriptSig, and one output with empty scriptPubKey.
  • For some odd reason, historically there have been 29k txes of 62 bytes, which are the majority of sub-100-byte txes. Picking randomly, they appear to involve a miner kindly removing nonstandard UTXOs that were spammed at an empty scriptPubKey.
  • I myself made a similar 63-byte tx today and broadcasted it via Electron Cash. I believe this is the shortest standard relay-able tx that can be made.
  • I do have a (somewhat contrived) use case in mind, which involves wallet users sending 540 sats to a short anyone-can-spend P2SH, then also broadcasting a short ~66 byte tx of this form to donate the 540 sat dust to miners. Such short P2SH would serve as a public notification point for clients attached to ElectrumX servers, who can then easily find all attached transactions by querying address history. (Note I have chosen an anyone-can-spend so as to not bloat the UTXO set.) It is possible to make this approach work with padding to 100 bytes but that's just less efficient space-wise.
@Mengerian

This comment has been minimized.

Copy link
Contributor

Mengerian commented Oct 17, 2018

I'd suggest adding:

  • Enforce "Minimal Data Push" (BIP 62 rule #3)
  • Enforce "Null Dummy" (BIP 62 rule #7)
* OP_INVERT
* OP_LSHIFT
* OP_RSHIFT
* Enable schnorr signatures

This comment has been minimized.

@schancel

schancel Oct 17, 2018

Author Collaborator

Per comments, make the following changes:

  • Add MINIMAL_PUSHDATA enforcement, and NULLDUMMY enforcement.
  • Remove <100 Byte TX limit, replace with 64byte tx exclusion
@paOol

This comment has been minimized.

Copy link

paOol commented Oct 17, 2018

support for non-standard scripts for things like BitcoinToken.

also, increasing DEFAULT_DESCENDANT_LIMIT, and/or the mempool issue to allow for more than 25 txs.

@sickpig

This comment has been minimized.

Copy link
Collaborator

sickpig commented Oct 17, 2018

@schancel

Enable schnorr signatures

Via leveraging the work done on libsecp? Or do you have something else in mind?

@jasonbcox

This comment has been minimized.

Copy link
Collaborator

jasonbcox commented Oct 17, 2018

Linking for posterity: https://twitter.com/micropresident/status/1052424498828468224
Shammah is also proposing a Feb. 15th spec deadline.

@floreslorca

This comment has been minimized.

Copy link

floreslorca commented Oct 17, 2018

Is there some document that indicates what is the process, and what would constitute a valid specification? Something like this

https://github.com/bitcoin/bips/blob/master/README.mediawiki

If it is, we should include it here

@schancel

This comment has been minimized.

Copy link
Collaborator Author

schancel commented Oct 17, 2018

support for non-standard scripts for things like BitcoinToken.

also, increasing DEFAULT_DESCENDANT_LIMIT, and/or the mempool issue to allow for more than 25 txs.

Transaction chain handling is actually quite expensive and has issues starting at 25 txns. Some seriously refactors need to be done to the code before I would want to bump that. There was problems at >25 txns. I'll have the individual looking into that post here.

@sickpig

This comment has been minimized.

Copy link
Collaborator

sickpig commented Oct 17, 2018

@schancel

Transaction chain handling is actually quite expensive and has issues starting at 25 txns. Some seriously refactors need to be done to the code before I would want to bump that. There was problems at >25 txns. I'll have the individual looking into that post here.

Any pointer that describe the problem with unconfirmed txns chain longer than 25?

@Mengerian

This comment has been minimized.

Copy link
Contributor

Mengerian commented Oct 17, 2018

I would propose the following timeline (or something similar):

  • 2019 Jan 01 - Specification Complete (date proposed by Mark Lundeberg)

  • 2019 Feb 01 - Code Complete

  • 2019 Feb 15 - Release of Upgrade Implementations including Activation

  • 2019 May 15 - Upgrade

Perhaps the different implementations have some ideas about this. If we want to make a release 3 months before the Update time, I think it makes sense to have deadlines prior to that for specification completion and code completion.

@schancel

This comment has been minimized.

Copy link
Collaborator Author

schancel commented Oct 17, 2018

@schancel

Transaction chain handling is actually quite expensive and has issues starting at 25 txns. Some seriously refactors need to be done to the code before I would want to bump that. There was problems at >25 txns. I'll have the individual looking into that post here.

Any pointer that describe the problem with unconfirmed txns chain longer than 25?

CPFP requires a full BFS every time you add or remove transactions from the mempool. It can get quite expensive. There was also some preliminary data showing a massive spike after a chain of 25; but I don't know the detailed explanation there yet.

My plan is to limit CPFP to a depth of 1, but allow chains to be arbitrarily long. It's a pretty substantial change though.

@dfoderick

This comment has been minimized.

Copy link

dfoderick commented Oct 17, 2018

@schancel

Transaction chain handling is actually quite expensive and has issues starting at 25 txns. Some seriously refactors need to be done to the code before I would want to bump that. There was problems at >25 txns. I'll have the individual looking into that post here.

The results of that study are here. https://docs.google.com/document/d/1cHZdMDJ2mhgRPnO9WuT_4qjYB15tLPmPFc10eLaBHBE/
"Performance Impacts of Raising the Ancestor and Descendant Transaction Limit for Bitcoin Cash"

Feedback is welcome.

Summary: The message we want to convey is that if node operators are willing to accept the performance impact of a modest increase in Tx limits now then it will help application developers and adoption. They can look at the graphs and decide what is acceptable to them. And anything that ABC can do to flatten the curve will be appreciated.

@dfoderick

This comment has been minimized.

Copy link

dfoderick commented Oct 17, 2018

@schancel

Transaction chain handling is actually quite expensive and has issues starting at 25 txns. There was also some preliminary data showing a massive spike after a chain of 25; but I don't know the detailed explanation there yet.

The spike I showed early in the study was not "real" but the conclusion is the same: Performance impact of increasing the tx limits is exponential, not linear.

Basically, I only saw the spike when I measured the rpc call to GetBlockTemplate. When I used the debug code then the spike went away but I was still left with exponential growth.

@schancel

This comment has been minimized.

Copy link
Collaborator Author

schancel commented Oct 19, 2018

Is there some document that indicates what is the process, and what would constitute a valid specification? Something like this

https://github.com/bitcoin/bips/blob/master/README.mediawiki

If it is, we should include it here

You can propose one. However, for the sake of "Decentralized Development." Several developers have refused to participate, or sign off, on such a process.

@markblundeberg

This comment has been minimized.

Copy link
Collaborator

markblundeberg commented Oct 19, 2018

Importing a comment from @DesWurstes on the discussion of last spec #94 .

Is it late for suggesting getting rid of hybrid 0x06, 0x07 P2PKH addresses' prefixes by considering those transactions temporarily nonstandard until making them invalid at some point?

I am going to suggest a hard fork making 0x06, 0x07 pubkeys completely invalid (ECDSA pubkeys must now start with 0x02, 0x03, or 0x04). Hybrid keys are silly, AFAIK nobody uses them (perhaps though they have been used once or twice in blockchain as a test), and from what I understand they are purely an artifact of bitcoin's historical dependence on OpenSSL.

@schancel schancel force-pushed the schancel:may-spec branch 3 times, most recently from 8302e76 to 3d86e3f Oct 19, 2018

activation: 1557921600
version: 0.1
author: Pieter Wuille
---

This comment has been minimized.

@schancel

schancel Oct 19, 2018

Author Collaborator

This document needs to be reformatted


## Schnorr Signatures

Support schnorr signatures per [schnorr-signatures.md](schnorr-signatures.md):

This comment has been minimized.

@schancel

schancel Oct 19, 2018

Author Collaborator

We need an additional specification for how the signature scheme will interact with bitcoin script.

This comment has been minimized.

@markblundeberg

markblundeberg Oct 19, 2018

Collaborator

Some questions to consider:

  • Are we making new opcode or overloading OP_CHECKSIG with new pubkey prefix bytes.
    • If new opcode, do we also make new standard P2PKH script or should all schnorr happen inside P2SH?
  • How should these mathematical objects be serialized?
@schancel

This comment has been minimized.

Copy link
Collaborator Author

schancel commented Oct 19, 2018

I've updated this to include several feedback items, and the opcode/schnorr specs.

@dgenr8

This comment has been minimized.

Copy link
Contributor

dgenr8 commented Nov 3, 2018

My plan is to limit CPFP to a depth of 1, but allow chains to be arbitrarily long. It's a pretty substantial change though.

@schancel Why discuss policy changes here? But your idea is easy to implement by expecting a child to pay for all of its ancestors, regardless of what its cousins are doing.

@awemany

This comment has been minimized.

Copy link

awemany commented Nov 3, 2018

It seems to me that the transaction validation code would get even simpler without any tx size limit. Given that it seems that new message types between FN<->SPV wallets can address the problem, it is IMHO a better idea to drop it completely as soon as the extra work to make it safe has been done on the network parts. That keeps the sensitive validation code as simple as possible.

Also, and as a general remark and brainstorming idea, I like to suggest that we look at various limits that we could simplify the rules for by relaxing them HF-wise sufficiently enough. Best would be if we could also remove more of the various activation code parts for past forks.

On enabling Schnorr sigs, I would like to caution that this should happen in a way that does not impact the viability of the ZCF scheme. Furthermore, ideal would be if it would equivalently work with Schnorr sigs as well. I am worried that there might ways to sign using the proposed Schnorr scheme but not triggering the double-spend detection as proposed using CDS-/V.

@Danconnolly

This comment has been minimized.

Copy link
Contributor

Danconnolly commented Nov 7, 2018

I'd like to propose a 4x increase in the generated and accepted block sizes for the May 2019 upgrade.

@markblundeberg

This comment has been minimized.

Copy link
Collaborator

markblundeberg commented Nov 7, 2018

On enabling Schnorr sigs, I would like to caution that this should happen in a way that does not impact the viability of the ZCF scheme. Furthermore, ideal would be if it would equivalently work with Schnorr sigs as well. I am worried that there might ways to sign using the proposed Schnorr scheme but not triggering the double-spend detection as proposed using CDS-/V.

Since every ECDSA keypair is also a valid Schnorr keypair, one way to do this is to simultaneously upgrade OP_CHECKSIG/V and OP_CHECKDATASIG/V to accept alternative signatures flagged for Schnorr processing (e.g., starting with a byte other than the usual DER 0x30). The ZCF scheme never really needs to care about how the data was signed, just 'it was signed by that key'.

Added advantage of this scheme is that you can simply flick a switch in your wallet software to start signing with Schnorr, and not even have to generate new addresses.

@AsashoryuMaryBerry

This comment has been minimized.

Copy link

AsashoryuMaryBerry commented Nov 8, 2018

Well, the BSV chain won't have CTOR.

@gasull

This comment has been minimized.

Copy link

gasull commented Nov 9, 2018

I'm surprised nobody is proposing adding protection against 51% attacks as described by @gavinandresen:

https://gavintech.blogspot.com/2012/05/neutralizing-51-attack.html

@cpacia

This comment has been minimized.

Copy link
Collaborator

cpacia commented Nov 9, 2018

@AsashoryuMaryBerry doh of course

@gandrewstone

This comment has been minimized.

Copy link
Contributor

gandrewstone commented Nov 9, 2018

responding to the proposed timeline: The proposal has a 2 week gap between code complete and release. This is far too short for the community to review this code and for other implementations to adopt it. Given that we are all busy with our own stuff, I think that its optimistic (and an inefficient use of time) to require that the community review work-in-progress code. The author should put the code into at least a "release candidate" state (that is, polished and containing test cases) before the community is asked to review, and whatever that date is should be identified in the timeline because basically that's when implementations need to clear their internal schedule in preparation for a lot of external work.

@awemany

This comment has been minimized.

Copy link

awemany commented Nov 10, 2018

@markblundeberg :

Since every ECDSA keypair is also a valid Schnorr keypair, one way to do this is to simultaneously upgrade OP_CHECKSIG/V and OP_CHECKDATASIG/V to accept alternative signatures flagged for Schnorr processing (e.g., starting with a byte other than the usual DER 0x30). The ZCF scheme never really needs to care about how the data was signed, just 'it was signed by that key'.

Yes that sounds good. My comment was rather of this sort: Let's be careful and think hard to not break existing use-cases. I just like to make sure that ZCF is thought of when the in-/validity rules for keys and sigs are being reworked.

@markblundeberg

This comment has been minimized.

Copy link
Collaborator

markblundeberg commented Nov 18, 2018

@Mengerian posted above:

I'd suggest adding:

* Enforce "Minimal Data Push" (BIP 62 rule #3)

I'd like to point out that this can be done without bricking any coins, so long as we restrict the minimal data push rule to ScriptSig alone (not applied when executing scriptPubKey, not applied when executing redeemScript).

In the long term we can effectively redefine the meaning of scriptSigs to be a list of data items that are uniquely encoded using Script minimal push opcodes. This list of data items represents the initial state of the stack before actual script execution starts.

Even already now because we have adopted the push-only rule, it is not necessary to execute scriptSig using EvalScript but rather we can just parse it as a list of data; old pre-fork transactions which may contain non-push opcodes can simply be blessed valid due to the checkpoint.

@HostFat

This comment has been minimized.

Copy link
Contributor

HostFat commented Nov 19, 2018

It would be great to have utxo commitments from @tomasvdw :)

@awemany

This comment has been minimized.

Copy link

awemany commented Nov 19, 2018

To echo @gandrewstone 's comments. I like to propose again (I have proposed this to some of you but not yet in public) that any feature that makes it into the hardfork has an implementation ready (maybe not in perfect final state but overall working and with tests!) for a common implementation, and I would argue for the time being it should be on any of the Satoshi-based C++ implementations.

I got feedback from some of you guys on this that this is "not how professional software development specifications" are made. Ahem.

But I want to take the opportunity again here to say that a) this is an argument from authority, b) I know of and actually have partaken in professional software development where not having code ready long before something launches is an absolute no-go and c) there's no reason to not try something new for Bitcoin, which is a new model of cooperation anyways. At least from my perspective it would help a lot in regards to getting legitimacy of a code change before the fork (people can play around with it), the solution space is explored better (e.g. for CTOR this would have meant to show that it actually helps, which is synergistic also with increased legitimacy!), specification bugs are found earlier and on top of that, it straightens out the release schedule and might reduce crunch-time quite a bit. Bugs have been found and fixed for the last fork, but it absolutely was rushed.

As a very minor example here, the idea to use only odd-sized arrays for Imaginary Username's blocksize proposal came to me only when I actually coded a sketch-up in python.

Specifically to ABC: You hopefully don't have to fight Faketoshis this time. Let's take it all a bit more thorough and slower.

@schancel

This comment has been minimized.

Copy link
Collaborator Author

schancel commented Nov 20, 2018

There's almost no commentary on the current text of the specification. I'd like to merge this and start having additional pull requests if possible

@schancel schancel self-assigned this Nov 20, 2018

@markblundeberg

This comment has been minimized.

Copy link
Collaborator

markblundeberg commented Nov 20, 2018

Re: current state

  • There are two copies of the re-enabled opcodes file.
  • It's still unclear why the OP_LSHIFT / OP_RSHIFT are defined in a noticeably different way from original functionality. Some discussion on reddit here.. Since these will be rarely used, I would suggest leaving them out and deferring to a later fork, until we can find a good reason to deviate from original function.
  • It is still unspecified how the Schnorr signatures are supposed to work as an opcode / how data are supposed to be encoded.
@Mengerian

This comment has been minimized.

Copy link
Contributor

Mengerian commented Nov 20, 2018

There's almost no commentary on the current text of the specification. I'd like to merge this and start having additional pull requests if possible

If you're going to merge it, I'd suggest making sure that it has a prominent disclaimer that's it is an "in progress" draft, still subject to change.

@markblundeberg

This comment has been minimized.

Copy link
Collaborator

markblundeberg commented Nov 24, 2018

Sorry to beat the dead horse but regarding LSHIFT/RSHIFT, here is a good example of their actual effect which seems to me to be undesirable. https://www.reddit.com/r/btc/comments/9z2shd/some_thoughts_about_op_lshiftop_rshift/eaca6h2/

@magmahindenburg

This comment has been minimized.

Copy link
Contributor

magmahindenburg commented Nov 28, 2018

I would like to raise the amount of chained unconfirmed transactions from 25 to a higher limit. It should have been raised with the block size limit. If it grows with the block size limit it would be 800. This can be done without a hard fork, but to be safe for on-chain services that relies on long chains of unconfirmed transactions the new setting should be activated at the same time on all full nodes.

@schancel

This comment has been minimized.

Copy link
Collaborator Author

schancel commented Nov 30, 2018

I would like to raise the amount of chained unconfirmed transactions from 25 to a higher limit. It should have been raised with the block size limit. If it grows with the block size limit it would be 800. This can be done without a hard fork, but to be safe for on-chain services that relies on long chains of unconfirmed transactions the new setting should be activated at the same time on all full nodes.

This is not consensus.

@magmahindenburg

This comment has been minimized.

Copy link
Contributor

magmahindenburg commented Nov 30, 2018

This is not consensus.

I know, but it's safer if all nodes activates the rules at the same time.

@schancel schancel force-pushed the schancel:may-spec branch 2 times, most recently from 5cfb392 to efd2851 Nov 30, 2018

@markblundeberg
Copy link
Collaborator

markblundeberg left a comment

Re: current state

There are two copies of the re-enabled opcodes file.
It's still unclear why the OP_LSHIFT / OP_RSHIFT are defined in a noticeably different way from original functionality. Some discussion on reddit here.. Since these will be rarely used, I would suggest leaving them out and deferring to a later fork, until we can find a good reason to deviate from original function.
It is still unspecified how the Schnorr signatures are supposed to work as an opcode / how data are supposed to be encoded.
[spec] 2019-05-15 Specification Draft
Add draft specifications for the 2019-05-15 protocol upgrade.

This commit also includes the nChain specification for reenabling
the following opcodes:
  * OP_MUL
  * OP_INVERT
  * OP_LSHIFT
  * OP_RSHIFT

@schancel schancel force-pushed the schancel:may-spec branch from efd2851 to cb61561 Dec 2, 2018

@schancel

This comment has been minimized.

Copy link
Collaborator Author

schancel commented Dec 2, 2018

@markblundeberg I've fixed the duplicate file. The other things should be hammered out in different PRs

@schancel

This comment has been minimized.

Copy link
Collaborator Author

schancel commented Dec 3, 2018

Merging. Please submit new pull requests to change various parts.

@schancel schancel merged commit 545cc02 into bitcoincashorg:master Dec 3, 2018

1 check passed

Staging (BitcoinCash.org) TeamCity build finished
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment