Skip to content

fix: TransactionRequest TryFrom fn #170

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

limyeechern
Copy link

@limyeechern limyeechern commented Apr 17, 2025

Summary

We need to decode the calldata using hex::decode for TransactionInput, otherwise estimate_gas that converts EvmTransactionData to TransactionRequest will fail

Testing Process

Checklist

  • Add a reference to related issues in the PR description.
  • Add unit tests if applicable.

Copy link

netlify bot commented Apr 17, 2025

Deploy Preview for openzeppelin-relayer ready!

Name Link
🔨 Latest commit 5fffba4
🔍 Latest deploy log https://app.netlify.com/sites/openzeppelin-relayer/deploys/6800cfb5d8940a000879012a
😎 Deploy Preview https://deploy-preview-170--openzeppelin-relayer.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

github-actions bot commented Apr 17, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@limyeechern
Copy link
Author

I confirm that I have read and hereby agree to the OpenZeppelin Contributor License Agreement

Copy link
Collaborator

@zeljkoX zeljkoX left a comment

Choose a reason for hiding this comment

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

hi @limyeechern

Thank you for improving the code and for your contribution!

@LuisUrrutia LuisUrrutia changed the title fix TransactionRequest TryFrom fn fix: TransactionRequest TryFrom fn Apr 17, 2025
@zeljkoX
Copy link
Collaborator

zeljkoX commented Apr 17, 2025

CLA assistant is failing, we have fix for this in pipeline

#171

@tirumerla
Copy link
Contributor

Hi @limyeechern - Thank you for your contribution. If you could rebase this to latest commit on main branch and ammend your commits to have verified signatures(signed commits ). We can merge it. Thanks once again

@limyeechern limyeechern force-pushed the lucas/fix_try_from branch from 193cf89 to a71207f Compare May 29, 2025 01:42
@limyeechern
Copy link
Author

hi @tirumerla, my commits have been signed and i have rebased to the latest commit on main. Thank you.

@tirumerla
Copy link
Contributor

tirumerla commented May 29, 2025

hi @tirumerla, my commits have been signed and i have rebased to the latest commit on main. Thank you.

Thank you for making the changes @limyeechern ...looks like there is a test that's failing if you could take a look that would be great.

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

Successfully merging this pull request may close these issues.

4 participants