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

decode fails at _decode_outputs sufficient length check throwing error on valid transaction #67

Open
willgriffin opened this issue Mar 2, 2015 · 3 comments · Fixed by #68

Comments

@willgriffin
Copy link
Contributor

$hex = "0100000001552eed137888e6a6c2c69ded505d9e573c3d78ab0f478ecbdaf74b99b40f350d010000006b483045022100d958e320b5bbc700e7862b7832fc86d18f50be7a272399c38b35a6aecd471d68022014a02f0387a0971c4e06cac086d662615a3e07a0323e1f138d96c54c7f6aaead012102af6034f808ee5989a7ea0304cc7d464edb22a86d362739aeb4e52e759436b7f5ffffffff0240480801000000001976a91415df9c5643a3ef61ee05a92a7703f47a4ffbbcdb88ac8b0361695e0000001976a91490967f997eda3a1c0bd4358b3cd19824e46538b688ac00000000";

RawTransaction::decode($hex, '6f', 'c4');

decoding works fine if i comment out the throw

@rubensayshi
Copy link
Member

hmm, you're right, @afk11 I think it should be substr($tx, 16, 2) in _decode_outputs because it's supposed to get the script length varInt.

but since its a varInt doing substr($tx, 16, 2) is not really correct either, see PR #68 to fix this

@afk11
Copy link
Member

afk11 commented Mar 9, 2015

Hmm, yeah that's not right. Depending on the usage it could die on a transaction it created itself. I'll have a look at properly decoding varints, as their length can be > 1 byte..

@afk11 afk11 closed this as completed in #68 Mar 9, 2015
@afk11 afk11 reopened this Mar 9, 2015
@afk11
Copy link
Member

afk11 commented Mar 9, 2015

Not closing this just yet as there still technically is a problem in how the library parses varints, I'll follow up with a PR for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants