search

bitcoinjs / bitcoinjs-lib

A javascript Bitcoin library for node.js and browsers.
MIT License
5.71k stars 2.11k forks source link

perfs: optimize script.decomplie return type #2097

Closed jasonandjay closed 6 months ago

jasonandjay commented 6 months ago

Optimize script.decomplie return type to Stack

  1. avoid type error. for example: null.map https://github.com/bitcoinjs/bitcoinjs-lib/issues/2096
  2. avoid unnecessary type assertion
  3. simplify the judgment of returned results
junderw commented 6 months ago

There is a semantic difference between [] and null

null indicates "the decompilation failed / there is no valid decompilation that exists for this Buffer" without need for throwing an exception.

[] means "we decompiled it and it was an empty script."

By collapsing this, [] can now mean two things.

This is a breaking change.

I agree that the bug in the other issue should be fixed, but this is not the way we should fix it.

Thanks for the PR though.

jasonandjay commented 6 months ago

There is a semantic difference between [] and null

null indicates "the decompilation failed / there is no valid decompilation that exists for this Buffer" without need for throwing an exception.

[] means "we decompiled it and it was an empty script."

By collapsing this, [] can now mean two things.

This is a breaking change.

I agree that the bug in the other issue should be fixed, but this is not the way we should fix it.

Thanks for the PR though.

I agree

How about throw error replace return null ?

junderw commented 6 months ago

I'd rather not have to think about the knock on effects.

A simple bug fix is easier to approve.

jasonandjay commented 6 months ago

@junderw how about this? just throw an error from toASM when decompile failed Minimal impact on existing logic

jasonandjay commented 6 months ago

Please help reRun actions it`s failed for netowrk issue, reRun failed works will repair