feat: Add l1 token address endpoint #1031
Conversation
| let base_token_addr = client | ||
| .get_base_token_l1_address() | ||
| .await | ||
| .context("Failed to fetch base token address")?; |
There was a problem hiding this comment.
This probably wouldn't work if the new EN is released before the server version is updated (e.g. EN will attempt to call this method, but it doesn't exist yet on the older version of server) -- and this is likely to happen, because EN is almost always released first, since we have to test the new version on testnet for some time.
Probably what we want to do is to handle the error: if the request fails with "method does not exist" error, we fallback to the existing behavior (base token is ETH). We still can fail on other errors.
There was a problem hiding this comment.
Hi @popzxc!.
I've handled the case for the missing rpc method here.
I'm not quite sure if this is the proper way to match with jsonrpsee's Error type, let me know if there's another and proper way to do it.
Also, one thing that to note is that I've hardcoded ETH's base token value here, I'm guessing this should be properly set as a const value somewhere, but I haven't found where this should be done.
There was a problem hiding this comment.
This is now set from the env, but the previous hardcoded value of0x0000000000000000000000000000000000000001 was because the zk init set the env var CONTRACTS_BASE_TOKEN_ADDR=0x0000000000000000000000000000000000000001
when not specifiying a non-eth base token
There was a problem hiding this comment.
@fkrause98 Not sure if I understand. We want to fallback to ETH if there is no dedicated config set for that (this may happen when we'll be rolling out this version of server on existing zkSync Era deployments). Why would we use 0x0000000000000000000000000000000000000001 in that case? We want to use ETH address, no?
There was a problem hiding this comment.
Done, now the ETH token case - with or without shared bridge - returns 0x...00
There was a problem hiding this comment.
BTW, I've consulted @vladbochok on this topic and it appears that this is still WIP, but we indeed leaning towards having it as 0x00..01 everywhere, including public interfaces.
This unfortunately renders my proposal incorrect -- sorry for that.
Now I believe that the right thing would be to make the base token optional, e.g. the corresponding config variable would have an Option<Address> instead of Address and if it's None then in the API we'll return Err(Web3Error::NotImplemented), otherwise we return whatever is stored in the config.
On EN we'll have two fallbacks then: first on MethodNotFound and second on NotImplemented, both resulting in setting base token to None. Otherwise it'll be Some whatever returned by API.
This way we'll have no additional hackery to support base token until it's explicitly enabled in existing envs.
Does it make sense?
There was a problem hiding this comment.
Yup, it does make sense, I'll make those changes and let you know.
There was a problem hiding this comment.
@popzxc I've implemented and tested the 3 cases you stated above and works as expected.
There was a problem hiding this comment.
@popzxc can we resolve this?
core/lib/zksync_core/src/api_server/web3/backend_jsonrpsee/namespaces/zks.rs
Outdated
Show resolved
Hide resolved
CI is failing in the destination branch. @kelemeno is working on that separately |
What ❔
This PR adds an RPC method that answers the chain's base token, or
0x0..01if the base token iseth.Why ❔
To make clients like block explorers or cli tools able to know which token the chain they point to is operating on, now that it could be one different from
eth.Checklist
zk fmtandzk lint.zk spellcheck.zk linkcheck.