Undoing the mixup between Chain ID and Network ID #777
Conversation
- There were validations in place, remove them - Stop returning the network id instead of the chain ID - Always return chain id is None until a new method is added to the json-rpc suite
web3/middleware/validation.py
Outdated
| @@ -15,14 +15,14 @@ | |||
|
|
|||
| @curry | |||
| def validate_chain_id(web3, chain_id): | |||
| if chain_id == web3.version.network: | |||
| if chain_id is None: | |||
There was a problem hiding this comment.
what?? This is going away?? Just because this middleware needed a web3 instance, I spent the last hour or two trying to figure out https://github.com/voith/web3.py/blob/b3a30a7374cb5f205d7e96a326533dc8394c7a73/web3/utils/formatters.py#L168-L180.
I would have got #756 right the first time if I knew this change was coming.
Also you might want to remove web3 from validate_chain_id(web3, chain_id) as its no longer needed
There was a problem hiding this comment.
I guess the clearer thing to do is his to set it to:
if chain_id == web3.net.chainId:Right now, that's the same thing. But once we get nodes to make chainId available in a standard way, it should start working again.
I'll review #756 today.
| False, | ||
| marks=pytest.mark.xfail, |
There was a problem hiding this comment.
@carver why did you mark this as xfail?
from web3.auto import w3
transaction = {'to': w3.eth.accounts[0], 'chainId': 9999999, 'from': w3.eth.accounts[1]}
w3.personal.unlockAccount(w3.eth.accounts[1], 'passphrase')
w3.eth.sendTransaction(transaction)shouldn't the above code give ValidationError for chainId? In fact validation middleware doesn't seem to do what it is expected to do.
I'm not very sure if this is a regression but I'm fixing this in #756.
| @@ -33,7 +33,6 @@ def transaction_normalizer(transaction): | |||
|
|
|||
| def validation_middleware(make_request, web3): | |||
| transaction_validator = apply_formatters_to_dict({ | |||
| 'chainId': validate_chain_id(web3), | |||
There was a problem hiding this comment.
I see that this is the cause for using xfail!
Read the description in the PR. However If validate_chain_id is not needed anymore, why is the code left behind?
Is this something you plan to return back to
There was a problem hiding this comment.
Is this something you plan to return back to?
Yes. Once we can reliably discover the chainID from the connected node, then we should validate chainID again.
What was wrong?
web3py was returning
networkIdwhen asked forchainId.How was it fixed?
There is no quick/foolproof way to determine the value, and
Noneis valid in all cases, so we just return that.chainIdvalidations in place, remove themNonefromw3.net.chainIdCute Animal Picture