SIN should call VersionedData constructor #441
Conversation
Creating SINs was broken due to not calling the parent constructor, shich sets "converts" and "_encoding". I've fixed the problem and added tests that reveal the error.
ryanxcharles
changed the title
|
Ah, now I understand. The EncodedData constructor must now be run so that is sets converters and _encoding. That is required for .as to work. Although the SIN constructor does not call .as directly, it does call .encoding, which in turn runs .as. The end result is that anything that derives from EncodedData must either run the constuctor (which sets converts and _encoding), or you must set those things manually. |
|
Here's the troublesome diff, in SIN.js: I changed the line that edits the prototype on the fly for a SIN. However, .converters and ._encoding now need to be set on the object itself, which can be done by calling the constructor of EncodedData. |
...revert to previous change, since always calling the constructor of VersionedData may have unintended consequences. Instead, just set .converts and ._encoding, since they are no longer in the prototype and must be set on the object itself.
|
I've updated the PR. I think I now have a complete understanding of the problem. SIN used to rely on the existence of .converters and ._encoding in the prototype. They are no longer in the prototype. Furthermore, SIN no longer sets the prototype at all in the constructor. .converters and ._encoding need to be set somehow. One way to set them is to call the VersionedData constructor. However, running that constructor with inappropriate inputs may cause uintended consequences. The simplest solution is to just set them by hand in the SIN constructor. That is what my latest commit does. |
|
...another way to look at it is that EncodedData and friends are really confusingly programmed and should be refactored to be easier to understand and modify. |
SIN should call VersionedData constructor
|
@ryanxcharles can we bump the version and publish this to npm? this way i can go ahead and lock down bitauth to bitcore@0.1.32. |
|
Yep. Doing so now. |
|
100% tests are passing here. |
Fix error condition in fiat rate endpoint
Creating SINs was broken due to not calling the parent constructor, shich sets "converts" and "_encoding". I've fixed the problem and added tests that reveal the error. Not completely sure why this was working before, since as far as I can tell, SIN always should have been calling the parent constructor anyway