Skip to content
This repository has been archived by the owner on Jun 11, 2022. It is now read-only.

Support testnet #277

Merged
merged 15 commits into from Dec 4, 2018
Merged

Support testnet #277

merged 15 commits into from Dec 4, 2018

Conversation

edolstra
Copy link
Contributor

This updates the genesis hashes and adds support for non-AVVM balances and the NetworkMagic field in addresses.

It also improves get_block error handling a bit.

Copy link
Contributor

@NicolasDP NicolasDP left a comment

Choose a reason for hiding this comment

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

  • a change of the type to use for the NetworkMagic;
  • deprecate previous helpers and/or provide new function that takes the NetworkMagic as parameter;
  • where are the tests for this ?

@@ -62,6 +63,21 @@ impl cbor_event::Deserialize for ProtocolMagic {
}
}

pub type NetworkMagic = Option<i32>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather like to use a specific type for this:

pub struct NetworkMagic {
    NoneRequired,
    Expected(i32)
}

Some(*pm as i32)
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be easier to put it, in the configuration of the CLI and hermes.

@@ -181,25 +182,30 @@ impl cbor_event::de::Deserialize for StakeDistribution {
#[cfg_attr(feature = "generic-serialization", derive(Serialize, Deserialize))]
pub struct Attributes {
pub derivation_path: Option<HDAddressPayload>,
pub stake_distribution: StakeDistribution
pub stake_distribution: StakeDistribution,
pub network_magic: config::NetworkMagic,
// attr_remains ? whatever...
}
impl Attributes {
pub fn new_bootstrap_era(hdap: Option<HDAddressPayload>) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to update the parameter of these functions to add their NetworkMagic. Or at least to deprecate them (or enable them only for the tests maybe?) so we don't have to do too much rewrite everywhere...

@NicolasDP
Copy link
Contributor

NicolasDP commented Oct 3, 2018

@NicolasDP NicolasDP added this to In progress in cardano-rust via automation Oct 10, 2018
@NicolasDP NicolasDP added this to the V1.0 milestone Oct 10, 2018
@NicolasDP NicolasDP mentioned this pull request Oct 10, 2018
Instead of

  thread '<unnamed>' panicked at 'index out of bounds: the len is 0 but the index is 0'

you now get

  hermes::service: Sync failed: Requested block 81a965de1412623ccd1cb3664f4d61a6cb4b9d53b44d779ed918e87bf3493f02 does not exist
This was added recently to cardano-sl and is used on the testnet.
This is needed for testnet.
@edolstra
Copy link
Contributor Author

Some updates:

  • NetworkMagic is now an enum rather than Option<i32>.

  • The Attributes and ExtendedAddr constructors (and various address generating functions) now require a NetworkMagic argument. cardano-cli will need to be updated for this.

  • Added a test for encoding/decoding address with NetworkMagic.

let xpub = unsafe { c_xpubkey.as_ref() }.expect("Not a NULL PTR");
let ea = ExtendedAddr::new_simple(xpub.clone());
let ea = ExtendedAddr::new_simple(xpub.clone(), NetworkMagic::from(protocol_magic));
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for this PR. just out of curiosity or knowledge sharing. Did you know you could use Into for everytime you have From implemented ?

This allows you to write things like:

    let ea = ExtendedAddr::new_simple(xpub.clone(), protocol_magic.into());

I like the verbose way, less chance to get it wrong. Still less character too.

// Yes, this is an integer encoded as CBOR encoded as Bytes in CBOR.
let bytes = raw.bytes()?;
let n = RawCbor::from(bytes.bytes()).deserialize::<u32>()?;
network_magic = NetworkMagic::Magic(n);
Copy link
Contributor

Choose a reason for hiding this comment

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

haskell 😩

@sjmackenzie
Copy link
Contributor

We're looking for test net functionality, could you be a dear and merge this?

@edolstra
Copy link
Contributor Author

Synced with master. @NicolasDP @vincenthz Want to merge?

@NicolasDP
Copy link
Contributor

@edolstra I cannot merge. This branch does not compile

@sjmackenzie
Copy link
Contributor

great chaps, thanks

Copy link
Contributor

@NicolasDP NicolasDP left a comment

Choose a reason for hiding this comment

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

thanks

@NicolasDP NicolasDP merged commit 6e4b56e into input-output-hk:master Dec 4, 2018
cardano-rust automation moved this from In progress to Done Dec 4, 2018
@sjmackenzie
Copy link
Contributor

hazzah!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
cardano-rust
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants