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

fix: show first 4 bytes of transaction data in signing string (instead of 3) #2522

Merged
merged 2 commits into from
Feb 19, 2025

Conversation

antazoey
Copy link
Member

What I did

Transaction showed data like (3 bytes)

data: 0x307833...303762

but is now (4 bytes):

data: 0x30783334...30303762

fixes: #2519

How I did it

How to verify it

Checklist

  • All changes are completed
  • Change is covered in tests
  • Documentation is complete

@antazoey antazoey changed the title fix: 4 bytes instead of 3 fix: show first 4 bytes of transaction data in signing string Feb 19, 2025
@antazoey antazoey changed the title fix: show first 4 bytes of transaction data in signing string fix: show first 4 bytes of transaction data in signing string (instead of 3) Feb 19, 2025
Copy link
Member

@fubuloubu fubuloubu left a comment

Choose a reason for hiding this comment

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

sweet. now I can go "What is method_id 0x30783334 in etherscan?" really quickly

Copy link
Member

Choose a reason for hiding this comment

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

should we make this configurable? like displaying the full data vs. only displaying partial?

accounts:
  show_full_calldata: true  # defaults `false`

Copy link
Member Author

Choose a reason for hiding this comment

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

I am always done for using config to change behavior like that

Copy link
Member Author

Choose a reason for hiding this comment

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

ill make another PR for this feature

Copy link
Member Author

Choose a reason for hiding this comment

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

@antazoey
Copy link
Member Author

Question: what about also showing the actual method name? We should know it.

@antazoey antazoey merged commit a32e621 into ApeWorX:main Feb 19, 2025
20 checks passed
@antazoey antazoey deleted the fix/sign-message-bytes branch February 19, 2025 21:04
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 this pull request may close these issues.

Show first 4 bytes of data, not just 3
3 participants