Page MenuHomePhabricator
Differential D17871

[ecash-lib] Add `parseSlp`
Needs ReviewPublic
Actions

Authored by tobias_ruck on Fri, Mar 28, 16:21.

Details

Reviewers
Fabien
bytesofman
Group Reviewers
Restricted Project
Summary

Parses an SLP OP_RETURN Script.

See doc comment of parseSlp for details.

Test Plan

npm test

Diff Detail

Event Timeline

tobias_ruck created this revision.Fri, Mar 28, 16:21
Herald added a reviewer: Restricted Project. · View Herald TranscriptFri, Mar 28, 16:21
Harbormaster completed remote builds in B32844: Diff 53298.Fri, Mar 28, 16:24
teamcity added a comment.Fri, Mar 28, 16:24
Build Bitcoin ABC Diffs / Diff Testing (ecash-lib-tests) failed.

Tail of the build log:

  hdwallet.ts                  |   41.13 |    39.47 |   48.27 |   81.69 | ...02,106-112,126,137,171 
  hmac.ts                      |   50.63 |       75 |   57.14 |     100 | 1-19                      
  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-17                     
  initNodeJs.ts                |   54.54 |      100 |     100 |      80 | 10                        
  messages.ts                  |   48.68 |     37.5 |     100 |   97.22 | 67                        
  mnemonic.ts                  |   50.65 |       50 |   53.33 |   98.71 | 96                        
  op.ts                        |   48.99 |       50 |   54.54 |   96.05 | 7,76,124                  
  opcode.ts                    |    50.2 |    83.33 |     100 |     100 | 1                         
  pbkdf2.ts                    |   50.87 |    83.33 |   66.66 |     100 | 1                         
  script.ts                    |   46.78 |    52.94 |   45.16 |   85.96 | 11,36,41-42,50,55,74,88   
  sigHashType.ts               |   50.66 |    54.54 |   53.84 |     100 | 1-79                      
  tx.ts                        |   45.05 |    45.23 |   38.09 |   82.97 | ...14,123-125,144,156,176 
  txBuilder.ts                 |   11.11 |     12.5 |   13.63 |   22.82 | ...07,214,251-255,261-265 
  unsignedTx.ts                |    51.3 |       49 |   51.28 |    91.6 | ...73,262,266,289,345,357 
 src/address                   |   48.47 |    51.51 |   53.84 |   94.82 |                           
  address.ts                   |      50 |    52.27 |   51.61 |   94.73 | 5,12,36,60                
  legacyaddr.ts                |   45.78 |       50 |    62.5 |      95 | 73,105                    
 src/ffi                       |   43.26 |    25.74 |   34.86 |   43.63 |                           
  ecash_lib_wasm_bg_browser.js |       0 |      100 |     100 |       0 | 1                         
  ecash_lib_wasm_browser.js    |       0 |        0 |       0 |       0 | 3-681                     
  ecash_lib_wasm_nodejs.js     |   90.03 |    72.22 |    74.5 |   90.28 | ...48,364-382,454,526,602 
 src/io                        |   48.55 |     54.9 |   54.68 |   94.85 |                           
  bytes.ts                     |   50.81 |       70 |   52.94 |     100 | 1-73                      
  hex.ts                       |   48.05 |       50 |   55.55 |   94.11 | 50,58                     
  int.ts                       |       0 |        0 |       0 |       0 |                           
  str.ts                       |   53.84 |    83.33 |      60 |     100 | 1                         
  varsize.ts                   |   48.97 |    52.63 |      60 |      96 | 24                        
  writer.ts                    |       0 |        0 |       0 |       0 |                           
  writerbytes.ts               |   44.26 |    43.75 |   53.33 |   87.09 | 34,44,54,64               
  writerlength.ts              |   53.33 |    83.33 |   53.84 |     100 | 1                         
 src/payment                   |    44.9 |    37.03 |   53.48 |   85.91 |                           
  asn1.ts                      |    44.3 |     30.2 |   51.21 |   84.92 | ...10,316,322,348,379,433 
  index.ts                     |      60 |      100 |     100 |     100 |                           
  x509.ts                      |   52.38 |    83.33 |     100 |     100 | 1                         
 src/test                      |       0 |        0 |       0 |       0 |                           
  testRunner.ts                |       0 |        0 |       0 |       0 | 16-201                    
 src/token                     |   39.32 |    39.89 |   41.26 |   65.77 |                           
  alp.ts                       |       0 |        0 |       0 |       0 | 13-145                    
  common.ts                    |   52.63 |    83.33 |     100 |     100 | 1                         
  empp.ts                      |       0 |        0 |       0 |       0 | 11-28                     
  slp.parse.ts                 |   52.15 |    51.88 |   52.17 |    98.5 | 10,17                     
  slp.ts                       |   35.76 |    27.94 |      52 |   68.35 | 41-66,206,213-220         
-------------------------------|---------|----------|---------|---------|---------------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='1694']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='4571']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='490']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='1245']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='267']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='709']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='1664']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='3035']
##teamcity[blockClosed name='Code Coverage Summary']
/work/modules/ecash-lib/coverage/lcov-report /work/modules/ecash-lib /work/abc-ci-builds/ecash-lib-tests
/work/modules/ecash-lib /work/abc-ci-builds/ecash-lib-tests
Build ecash-lib-tests completed successfully
tobias_ruck updated this revision to Diff 53299.Fri, Mar 28, 16:29

rerun CI

Harbormaster completed remote builds in B32845: Diff 53299.Fri, Mar 28, 16:32
teamcity added a comment.Fri, Mar 28, 16:32
Build Bitcoin ABC Diffs / Diff Testing (ecash-lib-tests) failed.

Tail of the build log:

  hdwallet.ts                  |   41.13 |    39.47 |   48.27 |   81.69 | ...02,106-112,126,137,171 
  hmac.ts                      |   50.63 |       75 |   57.14 |     100 | 1-19                      
  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-17                     
  initNodeJs.ts                |   54.54 |      100 |     100 |      80 | 10                        
  messages.ts                  |   48.68 |     37.5 |     100 |   97.22 | 67                        
  mnemonic.ts                  |   50.65 |       50 |   53.33 |   98.71 | 96                        
  op.ts                        |   48.99 |       50 |   54.54 |   96.05 | 7,76,124                  
  opcode.ts                    |    50.2 |    83.33 |     100 |     100 | 1                         
  pbkdf2.ts                    |   50.87 |    83.33 |   66.66 |     100 | 1                         
  script.ts                    |   46.78 |    52.94 |   45.16 |   85.96 | 11,36,41-42,50,55,74,88   
  sigHashType.ts               |   50.66 |    54.54 |   53.84 |     100 | 1-79                      
  tx.ts                        |   45.05 |    45.23 |   38.09 |   82.97 | ...14,123-125,144,156,176 
  txBuilder.ts                 |   11.11 |     12.5 |   13.63 |   22.82 | ...07,214,251-255,261-265 
  unsignedTx.ts                |    51.3 |       49 |   51.28 |    91.6 | ...73,262,266,289,345,357 
 src/address                   |   48.47 |    51.51 |   53.84 |   94.82 |                           
  address.ts                   |      50 |    52.27 |   51.61 |   94.73 | 5,12,36,60                
  legacyaddr.ts                |   45.78 |       50 |    62.5 |      95 | 73,105                    
 src/ffi                       |   43.26 |    25.74 |   34.86 |   43.63 |                           
  ecash_lib_wasm_bg_browser.js |       0 |      100 |     100 |       0 | 1                         
  ecash_lib_wasm_browser.js    |       0 |        0 |       0 |       0 | 3-681                     
  ecash_lib_wasm_nodejs.js     |   90.03 |    72.22 |    74.5 |   90.28 | ...48,364-382,454,526,602 
 src/io                        |   48.55 |     54.9 |   54.68 |   94.85 |                           
  bytes.ts                     |   50.81 |       70 |   52.94 |     100 | 1-73                      
  hex.ts                       |   48.05 |       50 |   55.55 |   94.11 | 50,58                     
  int.ts                       |       0 |        0 |       0 |       0 |                           
  str.ts                       |   53.84 |    83.33 |      60 |     100 | 1                         
  varsize.ts                   |   48.97 |    52.63 |      60 |      96 | 24                        
  writer.ts                    |       0 |        0 |       0 |       0 |                           
  writerbytes.ts               |   44.26 |    43.75 |   53.33 |   87.09 | 34,44,54,64               
  writerlength.ts              |   53.33 |    83.33 |   53.84 |     100 | 1                         
 src/payment                   |    44.9 |    37.03 |   53.48 |   85.91 |                           
  asn1.ts                      |    44.3 |     30.2 |   51.21 |   84.92 | ...10,316,322,348,379,433 
  index.ts                     |      60 |      100 |     100 |     100 |                           
  x509.ts                      |   52.38 |    83.33 |     100 |     100 | 1                         
 src/test                      |       0 |        0 |       0 |       0 |                           
  testRunner.ts                |       0 |        0 |       0 |       0 | 16-201                    
 src/token                     |   39.32 |    39.89 |   41.26 |   65.77 |                           
  alp.ts                       |       0 |        0 |       0 |       0 | 13-145                    
  common.ts                    |   52.63 |    83.33 |     100 |     100 | 1                         
  empp.ts                      |       0 |        0 |       0 |       0 | 11-28                     
  slp.parse.ts                 |   52.15 |    51.88 |   52.17 |    98.5 | 10,17                     
  slp.ts                       |   35.76 |    27.94 |      52 |   68.35 | 41-66,206,213-220         
-------------------------------|---------|----------|---------|---------|---------------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='1694']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='4571']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='490']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='1245']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='267']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='709']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='1664']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='3035']
##teamcity[blockClosed name='Code Coverage Summary']
/work/modules/ecash-lib/coverage/lcov-report /work/modules/ecash-lib /work/abc-ci-builds/ecash-lib-tests
/work/modules/ecash-lib /work/abc-ci-builds/ecash-lib-tests
Build ecash-lib-tests completed successfully
teamcity added a comment.Fri, Mar 28, 16:34
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Failed tests logs:

====== CashTab Unit Tests: <SendXec /> rendered with params in URL Legacy params are not parsed as bip21 even if the bip21 param appears in the string ======
Error: expect(received).toHaveProperty(path, value)

Expected path: "disabled"

Expected value: true
Received value: false
    at Object.toHaveProperty (/work/cashtab/src/components/Send/__tests__/SendByUrlParams.test.js:461:55)
    at runNextTicks (node:internal/process/task_queues:60:5)
    at listOnTimeout (node:internal/timers:545:9)
    at processTimers (node:internal/timers:519:7)

Each failure log is accessible here:
CashTab Unit Tests: <SendXec /> rendered with params in URL Legacy params are not parsed as bip21 even if the bip21 param appears in the string

tobias_ruck updated this revision to Diff 53300.Fri, Mar 28, 16:36

fix \0 usage

Harbormaster completed remote builds in B32846: Diff 53300.Fri, Mar 28, 16:47
tobias_ruck published this revision for review.Fri, Mar 28, 16:47
bytesofman requested changes to this revision.Fri, Mar 28, 17:29
bytesofman added a subscriber: bytesofman.

How is this function expected to be used? For the most part, correctly parsing an SLP OP_RETURN is often irrelevant, since we lack the indexer data (e.g. a well-formed SEND OP_RETURN could be a burn; validity in general depends on the indexer).

modules/ecash-lib/src/token/slp.parse.ts
29 ↗(On Diff #53300)

see other typeof comment

46 ↗(On Diff #53300)

why typeof MINT_STR and not just MINT_STR ?

I think you can set constant values this way if that is what is expected. otherwise typeof MINT_STR would just be string right?

48 ↗(On Diff #53300)

same

60 ↗(On Diff #53300)

same

62 ↗(On Diff #53300)

same

87 ↗(On Diff #53300)

confirm typeof use

99 ↗(On Diff #53300)

i.e. here we are just using the constant, seems like this would also be what we want to do above

133 ↗(On Diff #53300)

mint vault has a different lokad right?

oh nm just a different type

modules/ecash-lib/src/token/slp.test.ts
174 ↗(On Diff #53300)

is this actually out of spec?

it's possible to mint tokens without a ticker -- but in this case, there's supposed to be a placeholder?

202 ↗(On Diff #53300)

same question as above -- what is the spec on hash in a genesis tx?

This revision now requires changes to proceed.Fri, Mar 28, 17:29
tobias_ruck requested review of this revision.Fri, Mar 28, 20:45
tobias_ruck added inline comments.
modules/ecash-lib/src/token/slp.parse.ts
46 ↗(On Diff #53300)

why typeof MINT_STR and not just MINT_STR ?

'MINT_STR' refers to a value, but is being used as a type here. Did you mean 'typeof MINT_STR'?ts(2749)

otherwise typeof MINT_STR would just be string right?

typeof MINT_STR is the string literal type "MINT"

48 ↗(On Diff #53300)

'SLP_FUNGIBLE' refers to a value, but is being used as a type here. Did you mean 'typeof SLP_FUNGIBLE'?ts(2749)

99 ↗(On Diff #53300)

isn't it better to define the type once and then refer to it? this way we know for sure it's all the same type instead of me relying on my hands typing BURN instead of BRUN

modules/ecash-lib/src/token/slp.test.ts
174 ↗(On Diff #53300)

Ticker can be the empty string, yes, but it must be there in some way

202 ↗(On Diff #53300)

can either be empty or 32 bytes, but not missing entirely

bytesofman added a comment.Fri, Mar 28, 21:43

Is it possible to have a "valid" SLP OP_RETURN string that exceeds the OP_RETURN byte limit?

modules/ecash-lib/src/token/slp.parse.ts
46 ↗(On Diff #53300)

ok nice, TIL

99 ↗(On Diff #53300)

yeah, issue is my misinterpretation of typescript, i thought typeof BURN_STR was 'string' and not BURN

so here -- should we also define an UNKNOWN?

192 ↗(On Diff #53300)

spec-related magic number

200 ↗(On Diff #53300)

spec-related magic number, use a const or reference the spec

In Cashtab I went as far as to specifically define an SlpDecimals type for integers 0-9 inclusive but imo that was a mistake...functions need to handle arbitrary number input and validate, as here.

209 ↗(On Diff #53300)

spec-related const should have spec reference or be a defined const

330 ↗(On Diff #53300)

potentially some value in keeping this as a const? it's basically second-nature to "know" txids are 32 bytes / 64 chars long but I'm not sure that is clear to people not in the weeds of crypto dev

345 ↗(On Diff #53300)

I can see this being a useful const for ecash-lib to export (e.g. SLP_MAX_OUTPUTS = 19), will need to be re-defined otherwise in ecash-wallet

tobias_ruck updated this revision to Diff 53305.Sat, Mar 29, 00:28

add consts to address review; also factor out nextMintOutIdx

tobias_ruck updated this revision to Diff 53306.Sat, Mar 29, 00:33

add .js suffix

Harbormaster completed remote builds in B32851: Diff 53305.Sat, Mar 29, 00:37
Harbormaster completed remote builds in B32852: Diff 53306.Sat, Mar 29, 00:44
bytesofman added inline comments.Sat, Mar 29, 04:07
modules/ecash-lib/src/token/slp.ts
27 ↗(On Diff #53305)

for the particular case of these constants ... i dunno if we can use _LEN when we mean bytecount. in JS it's particularly confusing since sometimes we are checking tokenId strings, and the length then would be 64 (or 40 for address hashes).

mb _BYTES

Fabien accepted this revision.Mon, Mar 31, 13:15
Fabien added inline comments.
modules/ecash-lib/src/token/slp.parse.ts
105 ↗(On Diff #53300)

What about SlpScript? OP_RETURN is a technical detail

Revision Contents
Changeset List

PathSize
modules/
ecash-lib/
src/
1 line
token/
32 lines
380 lines
665 lines
23 lines

Diff 53306

View Options

modules/ecash-lib/src/index.ts

Loading...
View Options

modules/ecash-lib/src/token/common.ts

Loading...
View Options

modules/ecash-lib/src/token/slp.parse.ts

Loading...
View Options

modules/ecash-lib/src/token/slp.test.ts

Loading...
View Options

modules/ecash-lib/src/token/slp.ts

Loading...