[WIP] Added new networks #157 #158
Conversation
|
|
nejcr
changed the title
nejcr
changed the title
|
@nejcr Thanks for working on this PR. For new networks like prepod and preview, networkId (also known as Network Tag) is still 0 as these are test networks. You can refer to this CIP for more details https://cips.cardano.org/cips/cip19/#networktag But, the Network class in cardano-client has an another field Network Id is used in Address generation and during transaction serialization (optional). Please refer to my other comments inline. |
There was a problem hiding this comment.
@nejcr Can you please also add few tests to include PREVIEW, PREPOD network ?
Basically we need to check if a developer pass the network as PREVIEW or PREPOD, the generated address should be similar to regular testnet (starts with addr_test). I think we can add few tests in AddressTest class or AccountTest class.
| @@ -34,6 +34,8 @@ public static String getPrefixTail(NetworkId network) { | |||
| String prefixTail; | |||
| switch (network) { | |||
| case TESTNET: | |||
| case PREVIEW: | |||
There was a problem hiding this comment.
We don't need PREVIEW and PREPOD here. Only TESTNET networkId is enough. So NetworkId enum will have two values TESTNET, MAINNET where TESTNET is for all testnets
| @@ -52,6 +54,10 @@ public static NetworkId getNetworkId(Network networkInfo) { | |||
| network = NetworkId.MAINNET; | |||
There was a problem hiding this comment.
I think we need to change this logic. In existing code, Network object is being compared but instead we should compare NetworkId of the network and return NetworkId.MAINNET or NetworkId.TESTNET
Some thing like
if (NetworkId.MAINNET.equals(networkInfo.getNetworkId()) return NetworkId.MAINNET;
else if //check for network TESTNET network id
else throw AddressRuntimeException
| @@ -10,4 +10,14 @@ public static Network testnet() { | |||
| Network testnet = new Network(0b0000, 1097911063); | |||
| return testnet; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
This looks good to me. You may want to change the local variable name in prepod & preview methods to prepod/preview instead of testnet.
| @@ -2,5 +2,7 @@ | |||
|
|
|||
| public enum NetworkId { | |||
| TESTNET, | |||
| MAINNET | |||
| MAINNET, | |||
| PREVIEW, | |||
There was a problem hiding this comment.
PREVIEW and PREPOD are not required as only two possible values for NetworkId.
| @@ -155,6 +155,14 @@ public Map serialize() throws CborSerializationException, AddressExcepion { | |||
| case TESTNET: | |||
| bodyMap.put(new UnsignedInteger(15), new UnsignedInteger(0)); | |||
| break; | |||
| case PREVIEW: | |||
There was a problem hiding this comment.
These are network Ids. So only 1, 0. This change is not required.
| @@ -298,7 +306,15 @@ public static TransactionBody deserialize(Map bodyMap) throws CborDeserializatio | |||
| int networkIdInt = networkIdUI.getValue().intValue(); | |||
| if (networkIdInt == 0) { | |||
| transactionBody.setNetworkId(NetworkId.TESTNET); | |||
| }else if (networkIdInt == 1) { | |||
| } | |||
There was a problem hiding this comment.
Same as above. This change is not required.
|
@nejcr Let me know if you need any additional info for this PR |
|
Thanks @nejcr for the changes. Looks good to me. Merging it to a temp branch for now. I will do a minor change to compare networkid instead of network obj in |
I added new networks, however, I am not deeply familiar with this lib, and if someone (@satran004) could provide me the correct integer values I will update it accordingly.
#157