bitcoin-php icon indicating copy to clipboard operation
bitcoin-php copied to clipboard

Published 20 hours ago • verified publisher icon Bit-Wasp

ZCash

Open Vasiliy-Bondarenko opened this issue 6 years ago • 3 comments

I've parsed ZCash blocks. It has different header and tx structure, not Bitcoin-compatible. It requires custom BlockFactory with custom BlockHeaderSerializer and TransactionSerializer. I made it work by extending this classes. The question is - how to better add this to this package.

In TransactionSerializer properties have private access, so i had to redefine whole class without extending. Why it's private instead of protected which would allow extension and redefining just single method?

If you don't want to spend time thinking too much on this now - i can make PR on my own mind.

Vasiliy-Bondarenko avatar Mar 30 '18 11:03 Vasiliy-Bondarenko

Hey!

Good questions here.

I've accepted PR's for address related constants and stuff because it's simple and unintrusive, but I don't want to widen the scope of this library beyond bitcoin.

I recently experimented with BCH and found some of the changes a bit intrusive for the library, and eventually decided abstraction is the way to go to make this happen. I think we can do the same with zcash as here: https://github.com/btccom/bitwasp-bitcoin-bch-addon

We can be more liberal about the features included there, but for now full support in this repo is a commitment I can't take on, but I'll certainly accept PR's for abstractions to get us there. So if you wanna PR something for TransactionSerializer please go ahead :)

afk11 avatar Mar 30 '18 12:03 afk11

I started the package here https://github.com/Vasiliy-Bondarenko/ZCash - can you take a look, please? Am i doing it right? By the way - in bitwasp-bitcoin-bch-addon you use the same class names as in main package. Why don't you prefix them like Bch_* or something? Does it has some advantages? To me it looks like using same class-names can be misleading.

Vasiliy-Bondarenko avatar Apr 03 '18 04:04 Vasiliy-Bondarenko

Mostly wanted to preserve the relative namespace path after the new root for familiarity, but nothing major. You can do use ... as ... to import them differently in userspace code (or not, if you're just using the bridge) use \Btccom\BitcoinCash\Address\AddressCreator as BchAddressCreator use \BitWasp\Bitcoin\Address\AddressCreator as BtcAddressCreator

Will do a quick review!

  • ZcashTransactionSerializer - I think zcash is based on bitcoin 0.12, so no ScriptWitnessSerializer necessary :)
  • BlockHeader and nonce, maybe with ZcashBlockHeader you can throw if they call getNonce? add getZcashNonce? It certainly won't work with bitcoins ProofOfWork class anyway
  • ZcashTransaction can encapsulate the join split key / sig and stuff.

afk11 avatar Apr 03 '18 11:04 afk11