bitcore icon indicating copy to clipboard operation
bitcore copied to clipboard

Fixed Script.fromASM support PUSHDATA

Open baryon opened this issue 1 year ago • 3 comments

Script.fromASM should support PUSHDATA, otherwise it will lead to incorrect chunks settings, which in turn makes the result of toBuffer incorrect.

The bitcore-lib-cash code was previously fixed. LTC and DOGE are also incorrect, if corrections are needed, submit a patch separately.

baryon avatar Oct 09 '24 08:10 baryon

The test results of CircleCI do not seem to be related to this commit.

 1 failing

  1) Coin Model
       should appropriately mark coins related to transactions that are in mempool, but no longer valid:

      AssertionError: expected -1 to equal -3
      + expected - actual

      --1
      +-3
      
      at Context.<anonymous> (test/integration/models/coin.spec.ts:123:38)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

baryon avatar Oct 09 '24 11:10 baryon

In the cash code, a constant is calculated every time https://github.com/bitpay/bitcore/blob/d7d518209422cf5c7772808cba25f11ba874e75f/packages/bitcore-lib-cash/lib/script/script.js#L716 , which is unnecessary. In my code, the value of the constant is explicitly stated. Additionally, cash does not provide test cases for this part of the code.

baryon avatar Oct 10 '24 01:10 baryon

In the cash code, a constant is calculated every time

https://github.com/bitpay/bitcore/blob/d7d518209422cf5c7772808cba25f11ba874e75f/packages/bitcore-lib-cash/lib/script/script.js#L716 , which is unnecessary. In my code, the value of the constant is explicitly stated. Additionally, cash does not provide test cases for this part of the code.

Constants are fine - I agree that your constants are better than the computed values in BCH lib. I was more noticing the linting. You have inconsistent spacing around the braces and we're trying to move away from using lodash (_), so opcodenum == null or (opcodenum === undefined if null is a valid value) is more preferable over _.isUndefined(opcodenum)

There are 3 PUSHDATA tests in the BCH lib that are relevant to this codeblock, but your additional test is welcome.

Lastly, all of your commits need to be signed, so you'll need to squash all your commits into a new one and force push.

kajoseph avatar Oct 23 '24 15:10 kajoseph