-
Notifications
You must be signed in to change notification settings - Fork 20.3k
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
[ethapi] Expose overrides to eth client as well #21738
Conversation
// a message call. | ||
// Note, state and stateDiff can't be specified at the same time. If state is | ||
// set, message execution will only use the data in the given state. Otherwise | ||
// if statDiff is set, all diff will be applied first and then execute the call | ||
// message. | ||
type account struct { | ||
type Account struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an internal packag,e you can't use it as an API surface, Go won't allow outside callers to access it. All in all internal is meant to be used internally by Geth.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh - that's right - okay so I either move it out or make a same type in ethclient.go - i wonder if a type alias would do the trick here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just tried - a type alias worked great - I make an account in outside go-ethereum code just fine
internal/ethapi/api.go
Outdated
var accounts map[common.Address]account | ||
func (s *PublicBlockChainAPI) Call( | ||
ctx context.Context, args CallArgs, | ||
blockNrOrHash rpc.BlockNumberOrHash, overrides map[common.Address]Account, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point of *map[common.Address]account
is to differentiate between mandatory an optional arguments in the RPC layer. Please revert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but if caller doesn't provide the map - anyway it will be nil, no?
afdcec0
to
23f511e
Compare
ah - seems like i need to also change up abigen because my bindings now not compiling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot change CallContract
because it is part of the stable API, and many other projects use this API.
The state overrides are not supported everywhere, and I would strongly prefer to not add this functionality to ethclient. But I also understand it's annoying having to wrap the geth-specific APIs yourself.
I think what we could do is to add a new package "ethclient/gethclient" that contains an RPC wrapper for all the Geth-specific functionality.
@fjl okay that's a good point - I agree. Is there an enumeration of the specific geth functionalities ? |
Not really, but you can check the https://geth.ethereum.org/docs/rpc/server site for info about the geth-specific RPC methods (see sidebar). |
@fjl woof - that looks like quite a bit of refactoring to get that out the door - maybe if there is a design desired - i can impl that - otherwise i might go in a direction ya'll don't agree with |
You don't need to add all the methods, just add the package and the method you want. If you need some design help, what I want is something that's basically like ethclient:
|
I'm still interested to add this 'gethclient' package, but don't have time to work on it right now. Closing this because we won't make the change in ethclient itself. |
after seeing it mentioned in a tweet - checked and saw ethclient doesn't expose overrides of contract calls (eth_call)
also changed the extra pointer - maps are already pointers