This repository has been archived by the owner on Jul 9, 2021. It is now read-only.
Change all instances of networkId to chainId
#2313
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These should have been added back when we started generating these wrappers.
All of the contract artifacts were removed from the Python package recently, because now they're copied from the monorepo/packages area as an automated build step. Somehow this one artifact slipped through the cracks.
This was preventing the Exchange wrapper from ever importing its validator!
- Capture stderr (and have it included in stdout) so that it doesn't leak onto the console for commands that didn't actually fail. - Include all error output in the Exception object (eliminate print statement).
Newer versions care about this stuff. Old versions didn't, and we don't either.
`bytes.fromhex(bytes.decode('utf-8')` is just plain wrong. It would
work for some cases, but is not working when trying to fill orders with
the latest Exchange contract.
I swear the previous way was working before, but it wasn't working now, so this fixes it.
In various places. This allows the caller to install middleware (which in web3.py is installed on a Web3 object, not on a provider) before executing any RPC calls, which is important for the case where one wants to produce signatures locally before submitting to a remote node.
This allows easier consumption by other languages. (Specifically, it eliminates the overhead of keeping the Python addresses package in sync with the TypeScript one.)
Removed script that existed only to exclude runs of sra_client builds (parallel_without_sra_client). Now `parallel` is used by CI, re-including sra_client in CI checks.
It seems this hadn't been done since the merge with the 3.0 branch.
|
feuGeneA
requested review from
abandeali1,
albrow,
BMillman19,
fragosti,
hysz and
steveklebanoff
as code owners
albrow
suggested changes
Nov 4, 2019
10 tasks
fabioberger
approved these changes
Nov 5, 2019
| }; | ||
| const urlWithQuery = `${url}?assetDataA=${assetData}&networkdId=42&page=3&perPage=50`; | ||
| const urlWithQuery = `${url}?assetDataA=${assetData}&chainId=42&page=3&perPage=50`; |
There was a problem hiding this comment.
@dekz can you change the 0x API SRA endpoint to use chainId instead of networkId? We also need to update to SRA V3 spec with this change.
| @@ -138,15 +139,16 @@ describe.skip('CoordinatorWrapper', () => { | |||
| }, | |||
| }, | |||
| NETWORK_ID_TO_CONTRACT_ADDRESSES: { | |||
| [config.networkId]: contractAddresses, | |||
| // TODO: change to CHAIN_ID_TO_CONTRACT_ADDRESSES when @0x/coordinator-server is ready | |||
There was a problem hiding this comment.
Did you open up an issue for this so we can track it?
There was a problem hiding this comment.
@hysz would be good to get this on your radar
Thanks to @steveklebanoff for catching this. #2284 (review)
albrow
approved these changes
Nov 6, 2019
feuGeneA
force-pushed
the
chain-id-not-network-id
branch
from
72c7ce5 to
f92faaf
Compare
feuGeneA
force-pushed
the
chain-id-not-network-id
branch
from
f92faaf to
d9a409c
Compare
dorothy-zbornak
pushed a commit
that referenced
this pull request
Feb 27, 2020
* abi-gen/test: recompile contract fixtures for 3.0 It seems this hadn't been done since the merge with the 3.0 branch. * Sync `monorepo$ yarn test` exclusions to CI config * sra-spec: correct typo * contract-wrappers: TODO after coord.-server update * utils: fix typo in comment * Refactor networkId to chainId everywhere * Update CHANGELOGs
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Merge base chosen just for ease of review, because this PR builds on top of
@0x/contract-addresseschanges in that #2284, which is based on that branch.The
test-publishCI check is currently failing because@0x/mesh-rpc-clientis out of sync with this PR; specifically,@0x/mesh-rpc-clientis importingContractNetworksfrom@0x/types, but that interface has been renamed toContractChains. When@0x/mesh-rpc-clientis updated, we can simply re-run that CI job and then see all the pretty ✔️'s in here.Testing instructions
CI!
Types of changes
Checklist: