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

Store deployment info #320

Merged
merged 5 commits into from
Oct 11, 2018
Merged

Store deployment info #320

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7d0b9b3
Store deployment information and results
loredanacirstea Oct 10, 2018
0c071a2
Add post-deployment verification
loredanacirstea Oct 10, 2018
320c1dc
Add rinkeby deployment data.
loredanacirstea Oct 10, 2018
b878613
Code style fixes
loredanacirstea Oct 10, 2018
2a97dc2
Remove ChainID enum, keep dicts only
loredanacirstea Oct 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 6 additions & 22 deletions raiden_contracts/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,30 +97,14 @@ class MessageTypeId(IntEnum):
# Network configurations
START_QUERY_BLOCK_KEY = 'DefaultStartBlock'


class ChainId(IntEnum):
MAINNET = 1
ROPSTEN = 3
RINKEBY = 4
KOVAN = 42
SMOKETEST = 627


MAINNET = 'mainnet'
ROPSTEN = 'ropsten'
RINKEBY = 'rinkeby'
KOVAN = 'kovan'
SMOKETEST = 'smoketest'

ID_TO_NETWORKNAME = {
ChainId.MAINNET: MAINNET,
ChainId.ROPSTEN: ROPSTEN,
ChainId.RINKEBY: RINKEBY,
ChainId.KOVAN: KOVAN,
ChainId.SMOKETEST: SMOKETEST,
1: 'mainnet',
Copy link
Contributor

@LefterisJP LefterisJP Oct 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

But can you make them also constants?

e.g.:

NETWORKID_MAINNET = 1
NETWORKID_ROPSTEN = 3
...

e.t.c.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are they used somewhere? My thought was that ID_TO_NETWORKNAME and NETWORKNAME_TO_ID should be enough to do the job and I did not want redundant variables inside that file.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would allow us in the client to refer to ropsten's (or any other network's) ID programmatically without going through the ID_TO_NETWORKNAME whenever we want network specific behavior. It's just a convenience to avoid a mapping lookup.

And constant variables are not redundant in constants.py :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And constant variables are not redundant in constants.py :)

My problem is defining constants in raiden-contracts that are not used in raiden-contracts. If they are used only in raiden, then raiden should define them. Otherwise, we can break things by mistake.
To be honest, I would not even use raiden_contracts.ID_TO_NETWORKNAME in raiden... because of this. I would only stick to the raiden_contracts specific deliverables: contracts source, compiled files, ContractManager and the specific contracts-related constants.

But if you really want them added, I will.

Copy link
Contributor Author

@loredanacirstea loredanacirstea Oct 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

By the way, the entire ID_TO_NETWORK_CONFIG will be removed in the PR that solves #321
We will have json files with deployment data. So, any raiden specific network configs will be handled by raiden anyway.
The contracts will just provide a way to get deployment info for a specific version & chain.

Copy link
Contributor

Choose a reason for hiding this comment

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

My problem is defining constants in raiden-contracts that are not used in raiden-contracts.

Last time we discussed I had understood you wanted contract deployment related data to be in contracts. It used to be in raiden constants.py but then got moved here.

I really have no strong opinion on this. Just finalize these as quick as possible so that the client code has time to adapt. Whatever you remove from here, we will have in the client.

The contracts will just provide a way to get deployment info for a specific version & chain.

So will the contracts package provide a method for us to read this json file or will we have to implement it in the client?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Last time we discussed I had understood you wanted contract deployment related data to be in contracts

Correct. This has not changed. raiden_contracts will provide all contracts deployment info.

So will the contracts package provide a method for us to read this json file or will we have to implement it in the client?

I was thinking of adding in raiden_contracts a method for reading the json file based on version (default=current version) and the chain id, to make it easier for clients. Does this sound ok?

Copy link
Contributor

Choose a reason for hiding this comment

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

a method for reading the json file based on version (default=current version) and the chain id, to make it easier for clients. Does this sound ok?

I think it does. These 2 pieces of information is what the client will have. At the moment we also have this development | release configuration as additional data. I guess we can incorporate that in the version?

Copy link
Contributor Author

@loredanacirstea loredanacirstea Oct 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

development | release

Not sure what this is about. Are you talking about the pre-limits version and current version of the contracts? For this, I will have 2 versioned data dirs with the compiled + deployment info. E.g. data_0.4.2 for current one, and data_[PRE_LIMITS_VERSION] (I'll check what version that was) or something similar (I have to recheck the bumpversion release process, to see how this would work)

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you talking about the pre-limits version and current version of the contracts?

Yes exactly.

Your proposal sounds fine then. And we can make the choice based on the version in the client then, using the tool that you provide.

3: 'ropsten',
4: 'rinkeby',
42: 'kovan',
627: 'smoketest',
}


NETWORKNAME_TO_ID = {
name: id
for id, name in ID_TO_NETWORKNAME.items()
Expand All @@ -133,7 +117,7 @@ class NetworkType(Enum):


ID_TO_NETWORK_CONFIG = {
ChainId.ROPSTEN: {
3: {
NetworkType.TEST: {
'network_type': NetworkType.TEST,
'contract_addresses': {
Expand Down
2 changes: 1 addition & 1 deletion raiden_contracts/deploy/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ def verify_deployed_contracts(web3, contract_manager, deployment_file_path: Path
# Check that the deployed bytecode matches the precompiled data
blockchain_bytecode = web3.eth.getCode(endpoint_registry_address).hex()
compiled_bytecode = contract_manager.contracts[CONTRACT_ENDPOINT_REGISTRY]['bin']
# Compiled code contains some additional initial data compare to the blockchain bytecode
# Compiled code contains some additional initial data compared to the blockchain bytecode
compiled_bytecode = compiled_bytecode[-len(blockchain_bytecode):]
compiled_bytecode = hex(int(compiled_bytecode, 16))
assert blockchain_bytecode == compiled_bytecode
Expand Down