Bit-Wasp/bitcoin-lib-php

BC break for v1.1.0

afk11 opened this issue · 12 comments

afk11 commented

A few things warrant bumping the version number as there will be a BC break. The break is mostly in the ordering of P2SH keys which is currently incorrect, and that amounts will be handled in Satoshis and not BTC units.

A small few other changes immediately come to mind, if anything else pops up @rubensayshi @btcdrak note them here

  • Simple string sorting for public keys: #48
  • Signature serialization + encoding checks should be refactored #49
  • Library should use Satoshis and not BTC amounts: #58
  • Library should use latest phpecc - #59
  • Use correct PUSHDATA opcodes #55
  • Replace silent occurences of return false with exceptions #61

actual PR for using ints instead of floats; #58

also I'd like to go and replace most places where we return false and throw exceptions.

and maybe add a custom exception class instead of \Exception?

maybe you can make a v1.1 branch and already merge the stuff that's gonna go in there, then we can do PRs onto that instead of onto master and have less conflicts in the end?

well the right thing to do is create a 1.0 branch (dont use v prefix for branches) so we can maintain 1.0.x and the new stuff goes into master.

okay, so @btcdrak @afk11 you guys agree to add a BitcoinLibException and start throwing it everywhere?

Generally a custom exception makes no sense. There are plenty semantic ones to choose in PHP.

Okay, but replacing all the silent return false stuff with exceptions you agree with right?

@rubensayshi Yes: false should only be returned when a function processes something and returns false on valid input. If invalid inputs were received or something unexpected happened, it should always be an exception. This allows errors to bubble up the API. Returning false when something fails is definitely semantically wrong.

afk11 commented

PR's #59 and #60 bring the library up to date with the latest phpecc commit and should be producing minimal pushdata ops.

I've added the throw exceptions issue to the list of tasks

@btcdrak we have a few releases along v1.0.x already. I'll create a branch called 1.0, tag it as v1.0.6, and when we merge these commits we'll start following 1.1.0

Another PR to throw exceptions as much as possible; #61

afk11 commented

Nice one! Looks like we have everything we wanted now. I'll merge these now and create one last branch of 1.0.

afk11 commented