Deprecate 'testnet' network id in favor of 'devnet' #1938
Conversation
…ead of testnet in instance and localblockchain
…onstants being generated for the signatures are unchanged
|
Will this change break the verification keys of the contracts deployed now on devnet, including FungibleToken contracts? |
This is only a cosmetic change in order to agree on a common name for the network throughout the codebases. Under the hood we still use the same prefixes (unless its a custom network id) |
| @@ -44,7 +44,7 @@ export { | |||
| }; | |||
|
|
|||
| const networkIdMainnet = 0x01n; | |||
| const networkIdTestnet = 0x00n; | |||
There was a problem hiding this comment.
This is where constants used during signing are set, changing the networkId to devnet will not change the underlying constants so the signatures themselves will not be changed
There was a problem hiding this comment.
One thing we could consider in the future would be defining constant types and using those types throughout the code instead of string literals. Then we could update the actual string associated with the type more easily if we ever need to again.
const NetworkIds = {
Mainnet: 'mainnet',
Devnet: 'devnet',
/**
* @deprecated This network id is deprecated. Use `Devnet` instead.
*/
Testnet: 'testnet'
} as const;
type NetworkId = typeof NetworkIds[keyof typeof NetworkIds] | { custom: string }This also gives us a nice devX for deprecations:
| @@ -331,8 +331,9 @@ function getNetworkIdHashInput(network: NetworkId): [bigint, number] { | |||
| switch (s) { | |||
| case 'mainnet': | |||
| return [networkIdMainnet, 8]; | |||
| case 'devnet': | |||
| case 'testnet': | |||
There was a problem hiding this comment.
Can we add a deprecation warning here?
There was a problem hiding this comment.
Are you thinking a log statement or a comment? It's a bit noisy during tests with the log
There was a problem hiding this comment.
I was thinking a log... why is it noisy? Because we have tests still using testnet? I suppose we can do a very soft deprecation for now and continue to fully support testnet, but eventually we will want to remove it from our tests.
There was a problem hiding this comment.
no logs from library code please!
| @@ -91,6 +91,62 @@ let strings = [ | |||
| * - the 3 strings. | |||
| */ | |||
| let signatures: { [k: string]: { field: string; scalar: string }[] } = { | |||
| devnet: [ | |||
There was a problem hiding this comment.
nit: we could save a few lines here since devnet and testnet are the same payload anyway
…fix/update-network-id
Deprecates the
testnetnetwork id in favor ofdevnet. Functionality is unchanged andtestnetcan still be used with the same behavior asdevnet.