Stack template fixes

[dcousens]

The OP_0 / Buffer< > interchange is a nightmare.

I don't know if there is a solution other than minimizing interchange between Stack/Chunks/Script types, because the distinction is immediately lost without classification.

As for decoding from a stack... we can't make assumptions about whether it is OP_INT or Buffer.

[dcousens]

Gross

[dcousens]

This isn't turning out as elegant as I think we hoped.
The intrinsic types at play are [now]:

OP_INT = Int8
OP = Int8
Chunks = [OP_INT | OP | Buffer]
Script = Buffer
Stack = [Buffer]

check :: Script -> Bool
checkRaw :: Chunks -> Bool
checkStack :: Stack -> Bool

encode :: ... -> Script
encodeRaw :: ... -> Chunks
encodeStack :: ... -> Stack

decode :: Script -> ...
decodeRaw :: Chunks -> ...
decodeStack :: Stack -> ...

Witness inputs don't have a Script encoding... but they do have Stack

What we had previously:

check :: (Script | Chunks | Stack) -> Bool

encode :: ... -> Script
encodeStack :: ... -> Stack -- often used as Chunks
decode :: (Script | Chunks | Stack) -> ...
decodeStack :: (Stack | Chunks) -> ...

[dcousens]

The issue was decodeStack was returning Chunks as evidenced in #861

[dcousens]

Hopefully we can use asMinimalOP to avoid the OP_0 nightmare

[dcousens]

I think maybe decompile should decompile minimally? Maybe? Thoughts?
We need to be strict about this.

The question:

Should script.decompile return Buffer or OP_ if it is ambiguous

[afk11]

Should script.decompile return Buffer or OP_ if it is ambiguous

It should return OP_... and leave interpreting the opcodes up to the caller. The more information they have, the better :)

[afk11]

weak nack, I would avoid using opcodes as an intermediate (as encodeRaw seems to do) It would mean doing this: chunks -> stack -> scriptSig which doesn't make a good constraint, since not all chunks have a stack? (scriptPubKeys don't have the properties that allow a stack representation - they have opcodes and operate on a stack.. but scriptSigs do get evaluated and have a stack respresentation)

Whereas the following are invariant when SIGPUSHONLY and MINIMALDATA are in effect:

stack -> chunks -> scriptSig
stack -> witness

[dcousens]

I would avoid using opcodes as an intermediate

The opcodes are used intermediately as they are how those script templates are typically defined. Not as a stack?
For a witness stack, they are encoded to be a push-only vector of Buffers ([Buffer]).

It would mean doing this: chunks -> stack -> scriptSig

We have two typical flows for encoding...

Chunks -> Stack             -- a witness stack
Chunks -> Script             -- scriptSig, scriptPubKey

encodeRaw outputs Chunks, not a stack, nor a script.

---

I think the conflation of stack being anything other than the witness stack is confusing, and may be leading to our different approaches.
It has a specific meaning in bitcoin-core, and I don't think that is the meaning we should take on.
Maybe encodeStack should be encodeWitness...

[dcousens]

As mentioned on slack

• encodeStack to be deprecated, renamed as encodeRaw.
• toStack to be deprecated, renamed as toWitness.
• encodeWitness to be added.

Stack as a "type" in bitcoinjs-lib is to be removed (in 4.0.0) so we only have:

OP_INT = Int8
OP = Int8
Chunks = [OP_INT | OP | Buffer]
Script = Buffer
Witness = [Buffer]

check :: Script -> Bool
checkRaw :: Chunks -> Bool
checkWitness :: Witness -> Bool

encode :: ... -> Script
encodeRaw :: ... -> Chunks
encodeWitness :: ... -> Witness

decode :: Script -> ...
decodeRaw :: Chunks -> ...
decodeWitness :: Witness -> ...

compile :: Chunks -> Script
decompile :: Script -> Chunks
toWitness :: Chunks -> Maybe Witness

There is no fromWitness, you can't go back as contextual chunk information is lost due to ambiguity between OP_INT and Buffer.

An important distinction to understand is that chunks are semantic, contextual. For a chunk, OP_0 is not equal to Buffer < >.
And if you don't comply with minimal PUSH_DATA, they aren't equal.

Any deprecations to be removed in 4.0.0.
We may also re-structure for 4.0.0 to isolate Script encoding from Witness encoding, not under the same templates banner - but that is another conversation.

ping @afk11 ACK if you are on the same page

[dcousens]

This could be insanely simplified by removing the encoding to Script or Witness, and instead leave that to the user as an option.

bitcoin.script.p2pk.encode(pubKey) // existing
bitcoin.script.toScript(bitcoin.script.p2pk.encode(pubKey)) // replacement? booooo

// other ideas
bitcoin.p2pkh.fromKeyPair(kp).address
bitcoin.p2pkh.fromKeyPair(kp).script
bitcoin.p2pkh.fromAddress(address).script
bitcoin.p2pkh.fromOutputScript(script).address

// how validate??? try/catch?

// and then how redeem... ?
bitcoin.p2pkh.redeem(kp, signature).script
bitcoin.p2pkh.redeem(kp, signature).witness

bitcoin.p2pkh.redeem.fromScript(script)
bitcoin.p2pkh.redeem.fromWitness(witness)

// how check???

// what about reqSigs?

Above is related to #787
Hell, even #890

We need to be able to unify these two interfaces entirely, consistently, and powerfully to support simpler Transaction building with less verbosity and easy inspection.
Easy inspection via decode tooling will empower low-user-space-compelexity, verifiable smart contracts e.g lightning implementations.

edit: extra ideas from slack

let { maybe witness, maybe input, output } = p2sh(p2wsh(p2pkh(pubkey, maybe signature)))
let { maybe witness, maybe input, output } = p2sh(p2wsh(p2ms(pubkeys, maybe signatures)))
let { output } = p2sh(p2wsh(p2ms(pubkeys)))
let { witness, input } = p2sh(p2wsh(p2ms(pubkeys, signatures))) // fails without pubkeys, missing redeemscript
let { witness, input } = p2ms([], signatures)

let { witness, output, pubKeys, signatures } = p2ms({ witness, scriptPubKey })

[dcousens]

I think templates.witnessPubKeyHash and templates.witnessScriptHash may have been the wrong approach.

We may have needed instead, a plain witnessOutput instead, not a whole extra category.
For example, as of now, you can have an error thrown for a uncompressed pubkey in witnessPubKeyHash, but not in a pubKeyHash witness script!

Handling that in a scriptHash witness script (aka, not witnessScriptHash)... poop?

[dcousens]

Too many breaking changes, or potentially confusing ambiguities, and while this PR would make things stricter, it doesn't help the clumsiness of the API.

I think it will be better to simply move faster towards 4.0.0.