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.

Sign up for GitHub

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

Jump to bottom

fix: support for both input or data field in rpcCallRequest #4438

Closed
wants to merge 1 commit into from
Closed

fix: support for both input or data field in rpcCallRequest #4438

wants to merge 1 commit into from

Conversation

lsheva
Copy link

@lsheva lsheva commented Oct 4, 2023 •
edited

Issue:
go-ethereum from version 13.0 stopped working with local hardhat node. They did a change replacing data to input field in eth_call and other methods ethereum/go-ethereum#28078 referencing Ethereum specification

Update:
replaces rpcCallRequest to use input field instead of data. Fallbacks to data if input not set for backward compatibility.

@changeset-bot
Copy link

changeset-bot bot commented Oct 4, 2023

⚠️ No Changeset found

Latest commit: 37a20b9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Oct 4, 2023 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hardhat ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 4, 2023 7:46pm
hardhat-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 4, 2023 7:46pm

schmir added a commit to shutter-network/rolling-shutter that referenced this pull request Oct 5, 2023
@schmir
Update go-ethereum and abigen to v1.12.0
8d76f12 
Unfortunately we cannot use v1.13.0, as that breaks our tests. See
ethereum/go-ethereum#28078 and
NomicFoundation/hardhat#4438
@schmir schmir mentioned this pull request Oct 5, 2023
Merged
@rzmahmood rzmahmood mentioned this pull request Nov 21, 2023
Closed
@bilbeyt bilbeyt mentioned this pull request Dec 1, 2023
Open
@marlon-wiprud
Copy link

keeping an eye on this as its blocking usage of geth v1.13+ with hardhat

@marlon-wiprud marlon-wiprud mentioned this pull request Dec 6, 2023
Closed
@compojoom
Copy link

What needs to be done for this to get merged?
I spent half a day figuring out why my eth_calls were working fine against goerli nodes, but not against hardhat. Iit turns out that web3js by default uses input and I was ending with unrecognized selectors.

@fvictorio

fvictorio
fvictorio requested changes Dec 19, 2023
View reviewed changes
Copy link
Member

@fvictorio fvictorio left a comment

Choose a reason for hiding this comment

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

Hi everyone, and sorry for not responding before.

As it is right now, we cannot merge this PR for several reasons:

  • There are no tests
  • There doesn't seem to be an equivalent change for eth_sendTransaction (and I'm not sure if this change applies to eth_estimateGas; I think it does, but since there are no tests it's hard to know)
  • If we accept both input and data, then a call/gas estimate/tx using both should throw an invalid input error.

@jchappelow jchappelow mentioned this pull request Apr 3, 2024
Closed
1 task
@fvictorio fvictorio closed this Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fvictorio fvictorio fvictorio requested changes

@rzmahmood rzmahmood rzmahmood approved these changes

Assignees
No one assigned
Labels
status:triaging
Projects
Hardhat
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@lsheva @marlon-wiprud @compojoom @fvictorio @rzmahmood