-
Notifications
You must be signed in to change notification settings - Fork 237
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.
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
REGTEST Support for Mobile SDKs #1051
Comments
The main issue is that regtest mode is customizable. Just adding an enum case is not sufficient; you need a struct that can be configured with the regtest settings matching the full node you are using it with. And the available customization is not guaranteed to be the same across full nodes, so a one-size-fits-all type in these crates is not necessarily sufficient (but with the cooperation of relevant full node maintainers it might work).
enum Network {
Consensus(zcash_primitives::consensus::Network),
RegTest {
overwinter: Option<BlockHeight>,
sapling: Option<BlockHeight>,
blossom: Option<BlockHeight>,
heartwood: Option<BlockHeight>,
canopy: Option<BlockHeight>,
nu5: Option<BlockHeight>,
},
} |
Thanks @str4d for this insight. If I were to change the Network enum to match this I had another idea in mind but I'm not sure if it's possible in rust. pub enum Network {
MainNetwork,
TestNetwork,
RegTestNetwork(NetworkUpgradeValues),
}
pub struct NetworkUpgradeValues {
overwinter: Option<BlockHeight>,
sapling: Option<BlockHeight>,
blossom: Option<BlockHeight>,
heartwood: Option<BlockHeight>,
canopy: Option<BlockHeight>,
nu5: Option<BlockHeight>,
} I think that if this construct is possible in Rust, this change can be implemented with less impact in the whole codebase. |
closes zcash#1051 Also: Adds encoding tests + cargo fmt Add tests on ZIP-321 Adds coverage on new code
closes zcash#1051 Also: Adds encoding tests + cargo fmt Adds tests on ZIP-321 Adds coverage on new code
closes zcash#1051 Also: Adds encoding tests + cargo fmt Adds tests on ZIP-321 Adds coverage on new code
closes zcash#1051 Also: Adds encoding tests + cargo fmt Adds tests on ZIP-321 Adds coverage on new code
closes zcash#1051 Also: Adds encoding tests + cargo fmt Adds tests on ZIP-321 Adds coverage on new code
closes zcash#1051 Also: Adds encoding tests + cargo fmt Adds tests on ZIP-321 Adds coverage on new code
closes zcash#1051 Also: Adds encoding tests + cargo fmt Adds tests on ZIP-321 Adds coverage on new code
closes zcash#1051 Also: Adds encoding tests + cargo fmt Adds tests on ZIP-321 Adds coverage on new code
closes zcash#1051 Also: Adds encoding tests + cargo fmt Adds tests on ZIP-321 Adds coverage on new code Supports nu6 activation `local-consensus` feature is disabled by default
closes zcash#1051 Also: Adds encoding tests + cargo fmt Adds tests on ZIP-321 Adds coverage on new code Supports nu6 activation `local-consensus` feature is disabled by default
In order to get REGTEST support on the mobile SDKs, certain changes must be accommodated. I don't have a 100% certainty of what they are but I'll start pouring some of them here.
Changes to Librustzcash
zcash_primitives
Add an enum that represents Regtest distinctively
Provide Regtest variants to Parameters
pub trait Parameters: Clone
Make NetworkUpgrade Parameters variable.
REGTEST can be set up with different activation heights.
A RegTestNetwork struct should be able to provide these values according to the
environment that the caller is expecting.
Provide Regtest-specific
Parameters
trait valuesthe following values are constant and part of the
Parameters
traitzcash_client_backend
Add regtest encoding tests to
encoding.rs
add regtest examples to
zip321.rs
zcash_client_sqlite
no changes
zcash_extensions
no changes
zcash_history
no changes
Changes to
Components
zcash_address
I don't think changes are needed but this assertion on
zcash_address::Network::Regtest
might not hold after the changes in
zcash_primitives
zcash_encoding
No changes
Related GitHub Issues:
Electric-Coin-Company/zcash-swift-wallet-sdk#1233
Electric-Coin-Company/zcash-swift-wallet-sdk#1314
The text was updated successfully, but these errors were encountered: