Page MenuHomePhabricator

[ecash-lib] Add a bunch of helpers for `Script`
ClosedPublic

Authored by tobias_ruck on Thu, Apr 11, 23:01.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Commits
rABC2f2ee26da35a: [ecash-lib] Add a bunch of helpers for `Script`
Summary

Add helper functions fromOps, ops, cutOutCodesep, isP2sh, p2sh, p2pkh, p2pkhSpend to Script.

Add ScriptOpIter, which is returned by Script.ops to iterate over the Ops of a Script.

Test Plan

npm run test && npm run build

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

modules/ecash-lib/src/script.ts
64 ↗(On Diff #47062)

general question, how do you know we need this kind of function?

I'm not doubting it -- this is just super specific -- what is the use case where the need for this came up? Is it actually not specific and quite a general thing?

80 ↗(On Diff #47062)

why no corresponding isP2pkh?

modules/ecash-lib/src/script.ts
64 ↗(On Diff #47062)

My covenants utilize OP_CODESEPARATOR, so we need this function to sign BIP143 signatures using it.

There are multiple use-cases for OP_CODESEPARATOR, but the simplest is an optimization for BIP143-based covenants (using the OP_CHECKSIG + OP_CHECKDATASIG trick):

  • Really long Scripts will have a big BIP143 preimage, which costs precious bytes (and might go over the MAX_TX_IN_SCRIPT_SIG_SIZE).
  • This can be trimmed down to just one single byte by ending the covenant in ... OP_CODESEPARATOR OP_CHECKSIG, in which case the BIP143 signature algo will cut out everything after the OP_CODESEPARATOR, so only the OP_CHECKSIG. If the covenant bytecode is 520 or so, this would save 519 bytes.

There are other use-cases I found, like simplifying self-modifying covenants (see D15853), but those get a bit complex.

80 ↗(On Diff #47062)

It's actually never useful. The TxBuilder needs to handle P2SH different than all other kinds of scripts.

This also mirrors CScript in /src/script/script.h

modules/ecash-lib/src/script.ts
64 ↗(On Diff #47062)

The gist is that (IMO) every covenant should use OP_CODESEPARATOR for optimization, so this ends up being useful in the general case

Please add some more documentation and context to functions

modules/ecash-lib/src/script.ts
64 ↗(On Diff #47062)

This can be trimmed down to just one single byte by ending the covenant in ... OP_CODESEPARATOR OP_CHECKSIG, in which case the BIP143 signature algo will cut out everything after the OP_CODESEPARATOR, so only the OP_CHECKSIG. If the covenant bytecode is 520 or so, this would save 519 bytes.

My general concern with functions like this is that there is a lot of specialist knowledge behind them that is lost (or at least, not self-evident to a new maintainer or reviewer) without your personal experience.

This is an issue for the the whole lib until we have extensive documentation, which itself is its own job.

For now, please try to document these kinds of cases as best you can. Just sticking for example that whole paragraph in the comments is imo better than nothing.

Not a blocker, as having these methods available with no documentation is still much better than not having the methods available (and also no doucmentation). But something to keep in mind as this is built out.

eCash is already in a place where we have incredibly high horsepower dev tools with the broader problem being how to communicate this to devs.

80 ↗(On Diff #47062)

please document this in the function comment

This revision now requires changes to proceed.Fri, Apr 12, 12:59
modules/ecash-lib/src/script.ts
64 ↗(On Diff #47062)

Ok, good idea. It’s meant to be used internally mostly (by sigHashPreimage), but better to let people know the reason we have it exactly.

It’s a bit of a meme that the opcode is obscure and useless but I think we’ve found a lot of uses.

80 ↗(On Diff #47062)

👍🏻

bytesofman added inline comments.
modules/ecash-lib/src/script.ts
93 ↗(On Diff #47093)

would also be nice to have the discussion about why we do not need similar p2pkh method here

This revision is now accepted and ready to land.Fri, Apr 12, 17:35
modules/ecash-lib/src/script.ts
93 ↗(On Diff #47093)

Idk if we explain all the functions we didn’t put in it would be a long list