Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABCe348c74a4fdd: [ecash-lib] Add toHex method to Script class

Summary

I find myself frequently needing to do this manually, i.e. toHex(someScript.bytecode) ... it is simple enough manually but I also find myself always forgetting about .bytecode. Might as well have this in the lib.

Test Plan

npm test

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bytesofman created this revision.Tue, Jan 14, 19:41

Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Jan 14, 19:41

update changelog diff link

Harbormaster completed remote builds in B32085: Diff 52192.Tue, Jan 14, 19:58

Harbormaster completed remote builds in B32084: Diff 52191.Tue, Jan 14, 20:05

bytesofman published this revision for review.Tue, Jan 14, 20:12

Fabien added a subscriber: Fabien.Tue, Jan 14, 20:48

Fabien added inline comments.

modules/ecash-lib/src/script.ts
143 ↗	(On Diff #52192)	toHexString() ?

tobias_ruck added a subscriber: tobias_ruck.Tue, Jan 14, 20:50

tobias_ruck added inline comments.

modules/ecash-lib/src/script.ts
143 ↗	(On Diff #52192)	just `hex`

Fabien added inline comments.Tue, Jan 14, 20:52

modules/ecash-lib/src/script.ts
143 ↗	(On Diff #52192)	too pythonic

Fabien added inline comments.Tue, Jan 14, 20:53

modules/ecash-lib/src/script.ts
143 ↗	(On Diff #52192)	I don't mind hex(), toHex(), toHexString() but the point is that the hex is the important part here, not the string (this is the return type so it's obvious already)

bytesofman added inline comments.Wed, Jan 15, 00:39

modules/ecash-lib/src/script.ts
143 ↗	(On Diff #52192)	updated with `toScriptHex` ... which seems strange but is what we have in the Address class that does this same thing so I think this covers emphasis on hex, not string a bit "too specific' perhaps with Script.toScript ... but, well, we are converting this Script to ScriptHex in the same way we do so in the Address class `toHex` would be ideal but I think this is too easy to confuse with the existing ecash-lib exported function `toHex`

toString becomes toScriptHex

bytesofman retitled this revision from [ecash-lib] Add toString method to Script class to [ecash-lib] Add toScriptHex method to Script class.Wed, Jan 15, 00:40

update test name

Harbormaster completed remote builds in B32093: Diff 52200.Wed, Jan 15, 00:45

Build Bitcoin ABC Diffs / Diff Testing (ecash-lib-integration-tests) failed.

Tail of the build log:

File                        | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s            
----------------------------|---------|----------|---------|---------|------------------------------
All files                   |   30.09 |     28.5 |   31.07 |   47.71 |                              
 ecash-lib                  |       0 |        0 |       0 |       0 |                              
  eslint.config.js          |       0 |        0 |       0 |       0 |                              
 ecash-lib/dist/ffi         |       0 |        0 |       0 |       0 |                              
  ecash_lib_wasm_browser.js |       0 |        0 |       0 |       0 | 3-336                        
  ecash_lib_wasm_nodejs.js  |       0 |        0 |       0 |       0 | 1-264                        
 ecash-lib/src              |   35.78 |     28.6 |   38.75 |   67.94 |                              
  consts.ts                 |       0 |      100 |     100 |       0 | 6-8                          
  ecc.ts                    |   30.76 |    83.33 |   22.22 |   57.14 | 23-31                        
  hash.ts                   |   47.05 |    83.33 |   44.44 |   88.88 | 14                           
  index.ts                  |       0 |        0 |       0 |       0 |                              
  indexBrowser.ts           |       0 |        0 |       0 |       0 |                              
  indexNodeJs.ts            |       0 |        0 |       0 |       0 |                              
  initBrowser.ts            |       0 |      100 |       0 |       0 | 11-13                        
  initNodeJs.ts             |   53.84 |      100 |   66.66 |   85.71 | 12                           
  op.ts                     |   20.13 |    23.33 |   36.36 |   39.47 | ...4,107,109,117-122,133-161 
  opcode.ts                 |    50.2 |    83.33 |     100 |     100 | 1                            
  script.ts                 |   28.44 |    20.58 |   29.03 |   50.87 | ...2,133-144,150,160,182-193 
  sigHashType.ts            |      40 |       25 |   46.15 |   78.94 | 26-38                        
  tx.ts                     |   47.25 |    45.23 |   47.61 |   87.23 | 110,114,123-125,144          
  txBuilder.ts              |   40.64 |    32.69 |   54.54 |   80.21 | ...4,157,176-181,186,256-260 
  unsignedTx.ts             |   25.27 |       16 |   30.76 |   46.15 | ...9,312,320,326-329,345,357 
 ecash-lib/src/address      |   11.35 |    15.15 |    5.12 |   22.41 |                              
  address.ts                |   10.95 |    11.36 |    3.22 |   21.05 | ...3,239-240,255-256,266-344 
  legacyaddr.ts             |   12.04 |    22.72 |    12.5 |      25 | 15-19,23-38,70-111,124-128   
 ecash-lib/src/ffi          |   28.26 |    15.94 |   16.98 |   28.98 |                              
  ecash_lib_wasm_browser.js |       0 |        0 |       0 |       0 | 3-336                        
  ecash_lib_wasm_nodejs.js  |    61.9 |       55 |   39.13 |   62.75 | ...1,197-215,237,250-251,255 
 ecash-lib/src/io           |   30.45 |       41 |    38.7 |   58.77 |                              
  bytes.ts                  |    3.77 |     62.5 |    6.66 |     7.4 | 13-64                        
  hex.ts                    |   41.55 |       50 |   44.44 |   82.35 | 41-45,50,58                  
  int.ts                    |       0 |        0 |       0 |       0 |                              
  str.ts                    |   46.15 |    83.33 |      40 |   85.71 | 15                           
  varsize.ts                |   16.32 |    21.05 |      40 |      32 | 14-24,40-47                  
  writer.ts                 |       0 |        0 |       0 |       0 |                              
  writerbytes.ts            |   42.37 |    40.62 |   53.33 |   83.33 | 33,43,53,63,79               
  writerlength.ts           |   53.33 |    83.33 |   53.84 |     100 | 1                            
 ecash-lib/src/test         |    46.2 |    37.68 |      50 |   89.47 |                              
  testRunner.ts             |    46.2 |    37.68 |      50 |   89.47 | 73-75,88-89,112,123,166      
 ecash-lib/src/token        |   45.48 |    43.96 |      50 |   87.07 |                              
  alp.ts                    |    42.5 |    53.12 |   43.47 |   82.92 | 110-123,142                  
  common.ts                 |   54.54 |    83.33 |     100 |     100 | 1                            
  empp.ts                   |   52.17 |       60 |   57.14 |   91.66 | 12                           
  slp.ts                    |   46.97 |    33.82 |      52 |   89.74 | ...9,161,167,175,178,197,202 
----------------------------|---------|----------|---------|---------|------------------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='846']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='2811']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='258']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='905']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='142']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='457']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='826']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='1731']
##teamcity[blockClosed name='Code Coverage Summary']
mv: cannot stat 'test_results/ecash-lib-integration-tests-junit.xml': No such file or directory
Build ecash-lib-integration-tests failed with exit code 1

Harbormaster completed remote builds in B32094: Diff 52201.Wed, Jan 15, 00:55

I don't want to start bike shedding but this is the worst possible name. You can convert an address to script hex, that makes sense. But a script to script hex makes zero sense.
I also don't see how you can be confused with the toHex() function which is not a script method. That's what namespace is about.

Fabien requested changes to this revision.Wed, Jan 15, 08:59

This revision now requires changes to proceed.Wed, Jan 15, 08:59

toHex it is

bytesofman retitled this revision from [ecash-lib] Add toScriptHex method to Script class to [ecash-lib] Add toHex method to Script class.Wed, Jan 15, 13:38

Harbormaster completed remote builds in B32104: Diff 52212.Wed, Jan 15, 13:46

Fabien accepted this revision.Wed, Jan 15, 21:24

This revision is now accepted and ready to land.Wed, Jan 15, 21:24

Closed by commit rABCe348c74a4fdd: [ecash-lib] Add toHex method to Script class (authored by bytesofman). · Explain WhyWed, Jan 15, 21:29

This revision was automatically updated to reflect the committed changes.

bytesofman added a commit: rABCe348c74a4fdd: [ecash-lib] Add toHex method to Script class.

tobias_ruck added inline comments.Wed, Jan 15, 21:38