Skip to content

Conversation

@PivasDesant
Copy link

Found an issue where impersonated_hash was encoding the full transaction including the signature, which doesn't match how regular transaction hashing works in Ethereum.

Changed it to encode just the transaction data (without signature) first, then append the sender address - same pattern as the standard hash() method. This keeps things consistent with Ethereum standards while still making sure impersonated transactions get unique hashes even when they share the same fake signature.

Why this matters:

  • All impersonated txs use the same fake signature, so encoding with signature would give identical hashes for identical tx data
  • Ethereum tx hashing excludes signatures by design
  • Consistency with hash() makes the code easier to reason about

Found an issue where impersonated_hash was encoding the full transaction
including the signature, which doesn't match how regular transaction hashing
works in Ethereum.

Changed it to encode just the transaction data (without signature) first,
then append the sender address - same pattern as the standard hash() method.
This keeps things consistent with Ethereum standards while still making sure
impersonated transactions get unique hashes even when they share the same
fake signature.

Why this matters:
- All impersonated txs use the same fake signature, so encoding with signature
  would give identical hashes for identical tx data
- Ethereum tx hashing excludes signatures by design
- Consistency with hash() makes the code easier to reason about
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems alright but is also a bit pointless because this will never be correct encoding because we append the impersonated address as well.

but since this is "less wrong" I think we can do this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants