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

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

Details

Reviewers

bytesofman

Group Reviewers

Restricted Project

Commits

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

Repository

rABC Bitcoin ABC

Branch

ecash-lib-script-helpers

Lint

Lint Passed

Unit

No Test Coverage

Build Status

tobias_ruck requested review of this revision.Apr 11 2024, 23:01

modules/ecash-lib/src/script.ts
64	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	why no corresponding `isP2pkh`?

modules/ecash-lib/src/script.ts
64	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	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	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	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	please document this in the function comment

This revision now requires changes to proceed.Apr 12 2024, 12:59

modules/ecash-lib/src/script.ts
64	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	👍🏻