stdaddr: Introduce package for standard addrs.

[davecgh]

The current code for handling standard addresses implemented in dcrutil was written many years ago prior a wide variety of changes and several new features added by Decred. As a result, it entirely lacks support for some features and supports others in a roundabout and non-intuitive way.

Specifically, it does not support or provide a clean path to enable support for different script versions and the way they are handled in stake transactions is entirely non-intuitive.

Further, back when the original address code was implemented, it was necessary to implement script creation in the txscript package which led to the current design of providing methods such as txscript.PayTo{AddrScript,SStx,SStxChange}, and others, which need to type assert the specific concrete types of addresses in order to produce the necessary scripts. This, unfortunately, effectively negates the use of an interface to support generic addresses because it means callers, such as dcrwallet, are not able to implement their own types without somewhat invisibly breaking the script creation.

Finally, the aforementioned blending of the address code into txscript has led to confusion regarding what is considered standard and what is considered consensus which has tripped up several contributors over the years.

This aims to resolve all of the aforementioned issues by introducing a new package named stdaddr which entirely reworks the way addresses are handled.

It is split up into a series of commits to help ease the review process such that the initial generic infrastructure is introduced first, followed by the overall version 0 address decoding logic, then each supported version 0 address, followed by an example, and finally what I hope is a fairly complete README.md that describes the architecture. Comprehensive tests are included.

For the time being, the package is introduced into the internal staging area for initial review.

The following provides an overview of some of the key features of the new design:

• Supports versioned addresses
• Produces scripts directly via methods implemented on underlying types
• Provides direct support for creation of the scripts necessary for the staking system
• Uses a capabilities-based approach via interfaces so callers can cleanly and generically determine under what circumstances addresses can be used
• Allows callers to create their own concrete address types without worrying about breaking the existing ones
• Clearly denotes that addresses are a standardized construction that must not be used directly in consensus code

Finally, although the primary goal here is not optimization, this also significantly reduces the execution time and number of allocations needed to produce the payment scripts as can be seen in the following benchmarks:

Name	old time/op	new time/op	delta
p2sh	207ns ± 1%	39ns ± 1%	-81.24% (p=0.008 n=5+5)
p2pkh-ecdsa-secp256k1	219ns ± 2%	45ns ± 8%	-79.44% (p=0.008 n=5+5)
p2pkh-ed25519	235ns ± 2%	42ns ± 4%	-82.30% (p=0.008 n=5+5)
p2pkh-schnorr-secp256k1	232ns ± 1%	42ns ± 3%	-81.99% (p=0.008 n=5+5)
p2pk-ecdsa-secp256k1	276ns ± 1%	47ns ± 2%	-83.10% (p=0.008 n=5+5)
p2pk-ed25519	665ns ± 6%	47ns ± 1%	-92.98% (p=0.008 n=5+5)
p2pk-schnorr-secp256k1	283ns ± 1%	47ns ± 4%	-83.47% (p=0.008 n=5+5)

Name	old alloc/op	new alloc/op	delta
p2sh	512B ± 0%	24B ± 0%	-95.31% (p=0.008 n=5+5)
p2pkh-ecdsa-secp256k1	512B ± 0%	32B ± 0%	-93.75% (p=0.008 n=5+5)
p2pkh-ed25519	512B ± 0%	32B ± 0%	-93.75% (p=0.008 n=5+5)
p2pkh-schnorr-secp256k1	512B ± 0%	32B ± 0%	-93.75% (p=0.008 n=5+5)
p2pk-ecdsa-secp256k1	560B ± 0%	48B ± 0%	-91.43% (p=0.008 n=5+5)
p2pk-ed25519	640B ± 0%	48B ± 0%	-92.50% (p=0.008 n=5+5)
p2pk-schnorr-secp256k1	560B ± 0%	48B ± 0%	-91.43% (p=0.008 n=5+5)

[jrick]

This looks great.

May i suggest naming the package stdaddr? A little shorter and unlikely to collide with any variable names because those would probably read stdAddr when following the usual camel case style.

[davecgh]

This looks great.

May i suggest naming the package stdaddr? A little shorter and unlikely to collide with any variable names because those would probably read stdAddr when following the usual camel case style.

That sounds good to me. We also need to decide on where to put it. I just put it under internal/staging for now, but it will need to be exposed.

[matthawkins90]

I left two in-line comments, and then I have these three notes from my Readme review. I realize I'm not exactly the technical target audience for this package, but I'm trying to follow along for my own understanding. Here are my comments:

• You don't really define script in the Readme. I realize it's not really in the scope of this readme, but you asked for advice for how to make this primer more clear. Perhaps just some quick definitions or references would be helpful. I'm nominally aware of Bitcoin's Script, and aware that it uses opcodes for super concise and byte-efficient commands. That's the extent of my knowledge.
  • Is Decred's script the same as Bitcoin's, just with different opcodes?
  • You don't really talk about opcodes until Address Use in the Staking System
  • You use script instead of Script. The capitalization seems like it might be more clear that you're referring to the Script system.
• I don't know what Hash160 is, even after reading the blurb about RIPEMD-160 hashes. It would be nice to link to a reference.
• I don't know what you mean by Version 0 addresses. I don't know much about address versioning, unless you're talking about things like Segwit or Taproot.

Also, in the PR commit message, Paragraph 2 has a typo:
"Specifically, it does not support or provide a clean path to enable support for different script versions and it the way they are handled in stake transactions is entirely non-intuitive."

Otherwise, it reads very well, and the motivation and improvement is very clear! Thank you.

[davecgh]

I've updated this to rename the package to the recommended stdaddr and updated all references in the code, commits, and documentation accordingly.

[davecgh]

@matthawkins90 Thanks for the feedback. It's always good to get a fresh pair of eyes!

I'll comment inline, as your points are all valid, but, I did want to point out that, as you note, this is really aimed at developers who already have some background knowledge about the underlying concepts. If this were in a developer guide, I would put this type of information after sections which have already described how the transaction scripting language works which includes opcodes, as well as the notion of standard scripts.

You don't really define script in the Readme...

I'll add a link to the txscript package which should help clarify this I think.

Is Decred's script the same as Bitcoin's, just with different opcodes?

Yes.

You don't really talk about opcodes until Address Use in the Staking System

Right. The underlying transaction scripts are really just a collection of opcodes. There is some assumed knowledge here. Again, I think having an initial link to the txscript package near the top will help with this since it provides the prerequisite knowledge.

You use script instead of Script. The capitalization seems like it might be more clear that you're referring to the Script system.

Not sure I follow you here. Is there a specific instance you can point out? As far as I can tell, all of the references to scripts are indeed talking about scripts, whereas references to the scripting system specifically say scripting system. Maybe I missed some cases?

I don't know what Hash160 is, even after reading the blurb about RIPEMD-160 hashes. It would be nice to link to a reference.

Good point. I'll add a link to the ripemd160 package and clarify that Hash160 is shorthand for RIPEMD-160(BLAKE256(data)).

I don't know what you mean by Version 0 addresses. I don't know much about address versioning, unless you're talking about things like Segwit or Taproot.

That is likely because there currently is no real notion of versioned addresses since there is only the existing addresses (hence version 0). The plan in the future is to introduce a new version with a different encoding scheme. Also, perhaps more importantly, since addresses really just unpack to underlying scripts that enforce a set of constraints, and all software working with an address has to be able to produce scripts that are valid for a given script version, they effectively have a one-to-one correspondence with the underlying script version.

It seems like it would be worthwhile to add a blurb about address versioning in the primer.

Also, in the PR commit message, Paragraph 2 has a typo:

Fixed.

[davecgh]

@matthawkins90 I believe I have addressed all of you feedback. See the diff of changes here.

[matthawkins90]

Thanks for the updates. Reading now!
Meanwhile, Let me clarify what I meant by

You use script instead of Script.

Not sure I follow you here. Is there a specific instance you can point out?

Sorry for the confusion. This is incredibly nitpicky and inconsequential, so please let me know if this simply doesn't matter.
Here are my definitions:

1. Capitalized Script: the proper noun for the language system
2. Lowercase script: a stack of opcodes for a transaction.
3. txscript: dcrd's packaged implementation for Script.

The main phrase I'm getting hung up on is "script version".
I can't tell if you're talking about the Script language version, or if you intend to mean that the script of opcodes themselves can be versioned without versioning the Script.

So, examples:

• Line 20 - The script version of the aforementioned script
  Should probably read:
  - The Script version of the aforementioned script
  right?

• Line 23: It is also worth keeping in mind that the script version may be implicit
  Should be Script?

• Line 99: payment scripts that correspond to the same script version the original creator
  Should be Script version?

• Line 104

• Line 105

• Line 110
  ...
  I can do these in-line, but I want to make sure I'm not misunderstanding or wasting your time. This was simply something I noticed. Thanks for pointing me to the txscript package and answering my other questions!

[davecgh]

I've updated the generic decoding logic to implement a length check for determining if an address is probably a version 0 base58 address as well as slightly modified it a bit to be a bit more efficient when a new version is added as well as introduced some additional tests to cover the differences.

See Changes.

[davecgh]

@matthawkins90 Thanks for the clarification. I'll think on it a bit as I understand from your description why you find it confusing, though I'm not really sure that Script correctly refers to the scripting language because the current incarnation is named txscript. All scripts are accompanied by a script version (aka the scripting language version) that identifies the exact language and semantics the script must use. Currently, the only supported version is 0, but that is going to change in the future.

[dnldd]

Nicely done with great documentation 👍

[rstaudt2]

This looks great to me. The code is much easier to follow than the existing script creation code in txscript and it makes sense to cleanly split out standard script creation from txscript. The tests are excellent and make it straightforward to review the code for correctness as well. Just noticed a few typos.

[davecgh]

@matthawkins90 After thinking about it a bit, I decided to go with "scripting language version" throughout which should hopefully address your points. I also changed the wording from until absolutely necessary to until the funds are redeemed which hopefully resolves that point of confusion. Changes here.

[jholdstock]

Just reviewed the README. As somebody with a low-medium level of familiarity of the topic, it reads well and goes into a good level of depth. ACK.

[matheusd]

Excellent work! I also ran run_tests.sh in every commit to assert the correct behavior.

Only noted two points for clarification.

[matthawkins90]

@davecgh, really great work! Thanks for working to make this more approachable and understandable for people.

[matheusd]

🎉

[JoeGruffins]

Amazing.

[JoeGruffins]

This probably sounds wierd, but I can't get the example test to print out with go test --run=ExampleDecodeAddress -v unless I add another fmt.Println() somewhere else in the file. Not sure what it is.

edit: Oh, I see. It's not a test. Nevermind.

edit2: I cannot find much documentation on "package_test". Is it possible to have that print to stdout?

[davecgh]

This probably sounds wierd, but I can't get the example test to print out with go test --run=ExampleDecodeAddress -v unless I add another fmt.Println() somewhere else in the file. Not sure what it is.

It's a testable example. go test compiles, runs, and captures the output and then compares that output against the comments to ensure the example actually produces the output it claims to. That way they don't go stale as the code changes.

[JoeGruffins]

I see! I don't know how I missed this part of golang testing. I see that the output must match what is under // Output: Thank you.