Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zclassicjs: Unspendable Outputs because OP_NOP5 is NOT implemented in the reference client #194

Open
ghost opened this issue Dec 12, 2018 · 5 comments

Comments

@ghost
Copy link

ghost commented Dec 12, 2018

I was made aware of an issue where a user had used a ZenCash library against ZCL which rendered the Change output unspendable. As you can see, the transaction has been mined. Here's the transaction;

http://explorer.zclmine.pro/tx/678364244bffd778ba4659d657892f51c57db077813873fbaedd35d720c9991a

And here's the transaction script;

OP_DUP OP_HASH160 2352a7109c0e5db56d03c0cb7ea60f2e9b05c747 OP_EQUALVERIFY OP_CHECKSIG 0 0 OP_NOP5
Had this transaction been on ZenCash, the OP_NOP5 was implemented as OP_CHECKBLOCKATHEIGHT and the previous 2 values (0 , 0) would have been evaluated as the parameters ParamHeight and ParamBlockHash.

Since OP_NOP's are basically ignored when not implemented, as is the case with ZCL, then the last item on the script stack would have been 0, and therefore, I believe the output rendered unspendable.

I have asked around a little, and people seem to agree with my conclusion above. However, it has been raised that the outputs could possibly become spendable in the future. for example, if ZCL was to implement the replay protection (OP_CHECKBLOCKATHEIGHT) however, looking back through the history of ZCL it was actually removed in order to mirror ZCASH more closely.

There are obviously other changes or forks that can be done to make them spendable too.

Spending of the outputs theories could be tested by running a 1 node testnet.

It could be wise to analyse the chain to see how many of these transactions exist

The ideal outcome and closure of this ticket would be at minimum a proposal of how to spend the outputs in the future or now.

@nimbosa
Copy link
Member

nimbosa commented Jan 7, 2019

I am reading this just now, and it sounds like an interesting corner case scenario. @thedevworks

I have a few questions at the top of my head:

  1. is this a shielded transaction?

  2. why would someone use ZenCash software for spending on Zclassic network? was this for dev testing/ experimentation? or on production? this is an inherent risk in unimplemented features such as the one cited here, it would have been better to run the tx on testnet FIRST or write a test for it..

I will look more closely when I have the time. Feel free to contact me at the usual places, I am restoring my crypto communications channels one at a time (Telegram, Discord, etc..)

If anyone has more insight on this, please feel free to chime in and share your view. Thanks!

@ghost
Copy link
Author

ghost commented Jan 7, 2019

@nimbosa

  1. Nope, Clear
  2. They used this repo; https://github.com/z-classic/zclassicjs I guess it was never finished off

You are correct that they are entirely in the wrong, there should be at least minimum automated testing of making a transaction in a wallet before releasing to customers.. However, I was wondering if the outputs would ever become spendable again.

The wallet in question took the hit refunding the customer!

I suggest we remove that

@ghost
Copy link
Author

ghost commented Jan 7, 2019

Can you get back on keybase? I've been trying to get hold of you for weeks!

@nimbosa
Copy link
Member

nimbosa commented Jan 7, 2019

Thanks for the details @thedevworks I will get back on keybase later. ;)

I am not aware of the repo above and it is not a clear fork from ZenCashJS, so we have to manually check code change and its coverage. I would prefer making a public fork of a ZenCash/Zcash repo and start work from there so the difference would be clear and the results a little more predictable and easier to compare.

I hope this becomes a lesson for everybody. 🎉 🥂

@nimbosa
Copy link
Member

nimbosa commented Jan 7, 2019

found the source: https://github.com/ZencashOfficial/zencashjs

we can work two step backwards on this and maybe one step forward here in the reference client

I am inclined to move this to the right repo (https://github.com/z-classic/zclassicjs) as soon as a new issue or pull request comes up to resolve or implement this with specifics

@nimbosa nimbosa changed the title Unspendable Outputs caused by erroneos OP_NOP5 zclassicjs: Unspendable Outputs because OP_NOP5 is NOT implemented Jan 7, 2019
@nimbosa nimbosa changed the title zclassicjs: Unspendable Outputs because OP_NOP5 is NOT implemented zclassicjs: Unspendable Outputs because OP_NOP5 is NOT implemented in the reference client Jan 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant