fixes #80 #81
fixes #80 #81
Conversation
|
Changes Unknown when pulling 6fedafe on benjamincburns:issue80-hash-collisions into ** on ethereumjs:master**. |
fake.js
Outdated
| let items | ||
| if (includeSignature) { | ||
| items = this.raw | ||
| items += this.getSenderAddress(); |
There was a problem hiding this comment.
I think instead of duplicating, fakeTransaction should change .raw in the constructor to include the signature (a fake one perhaps). The hash method then is inherited unchanged from the normal implementation.
This is a lot of code duplication otherwise.
There was a problem hiding this comment.
@axic I think I can accommodate that easily enough. However, does that mean that instead of overriding hash we'll need to override sign?
There was a problem hiding this comment.
Also thanks for the fast response!
There was a problem hiding this comment.
@axic see latest changes. I'm not sure that I like this however, as I feel sign should probably be overridden as well to avoid cases where the fake signature is overwritten.
|
Changes Unknown when pulling 6326eb9 on benjamincburns:issue80-hash-collisions into ** on ethereumjs:master**. |
|
Changes Unknown when pulling d550ed8 on benjamincburns:issue80-hash-collisions into ** on ethereumjs:master**. |
|
Changes Unknown when pulling e6a795f on benjamincburns:issue80-hash-collisions into ** on ethereumjs:master**. |
|
Changes Unknown when pulling 00a47d8 on benjamincburns:issue80-hash-collisions into ** on ethereumjs:master**. |
|
Changes Unknown when pulling de8e066 on benjamincburns:issue80-hash-collisions into ** on ethereumjs:master**. |
|
@jwasinger well that's embarrassing! Pushing the fix now. Lesson learned: when testing with |
|
Changes Unknown when pulling f808209 on benjamincburns:issue80-hash-collisions into ** on ethereumjs:master**. |
|
Changes Unknown when pulling 25e0256 on benjamincburns:issue80-hash-collisions into ** on ethereumjs:master**. |
|
Changes Unknown when pulling 0aeba44 on benjamincburns:issue80-hash-collisions into ** on ethereumjs:master**. |
|
And a half hour later I've found the right mixture of whitespace to avoid offending |
|
lol isn't that always fun :D can you squash some of the commits to clean up the history? then I will merge. |
benjamincburns
force-pushed
the
issue80-hash-collisions
branch
from
0aeba44
to
b603bb2
Compare
|
squashed. |
|
Changes Unknown when pulling b603bb2 on benjamincburns:issue80-hash-collisions into ** on ethereumjs:master**. |
… for different senders
benjamincburns
force-pushed
the
issue80-hash-collisions
branch
from
b603bb2
to
7f24cf9
Compare
|
Changes Unknown when pulling 7f24cf9 on benjamincburns:issue80-hash-collisions into ** on ethereumjs:master**. |
|
Thanks, @jwasinger! |
|
@benjamincburns protip: |
|
Thanks, @kumavis - maybe I have a shitty old version installed, but it never seems to do anything for me -- it also wasn't showing the same errors locally as in CI. |
Fix ethereumjs#81 by checking it.value is valid
Fixes #80 - Don't produce hash collisions on FakeTransaction for different senders