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

Jump to bottom

Rename Floonet to Testnet #3431

Merged
merged 5 commits into from
Oct 7, 2020
Merged

Rename Floonet to Testnet #3431

merged 5 commits into from
Oct 7, 2020

Conversation

quentinlesceller
Copy link
Member

Fixes #3429.

This should also be compatible with grin-wallet too.

@quentinlesceller quentinlesceller marked this pull request as ready for review September 2, 2020 20:19
@quentinlesceller
Copy link
Member Author

This works as is

@antiochp
Copy link
Member

antiochp commented Sep 3, 2020

Awesome!
I am planning to test this out over the next couple of days.

@quentinlesceller quentinlesceller mentioned this pull request Oct 6, 2020
Merged
antiochp
antiochp reviewed Oct 7, 2020
View reviewed changes
Copy link
Member

@antiochp antiochp left a comment

Choose a reason for hiding this comment

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

Couple of minor comments. Couple of files should be renamed.

I want to spend a bit of time today testing this locally to make sure we generate fresh config and read existing config correctly etc.

etc/Dockerfile.floonet Show resolved Hide resolved
core/tests/consensus_floonet.rs Show resolved Hide resolved
keychain/src/keychain.rs
@@ -45,8 +45,8 @@ impl ExtKeychain {
}

impl Keychain for ExtKeychain {
fn from_seed(seed: &[u8], is_floo: bool) -> Result<ExtKeychain, Error> {
let mut h = BIP32GrinHasher::new(is_floo);
fn from_seed(seed: &[u8], is_test: bool) -> Result<ExtKeychain, Error> {
Copy link
Member

Choose a reason for hiding this comment

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

I have a WIP PR somewhere that attempts to cleanup passing around is_floo (now is_test). I'll see if I can resurrect it. I think we want to explicitly pass a "chain type" around rather than a simple bool like this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I leave it like that for now.

@quentinlesceller
Copy link
Member Author

Thanks for the review. Addressed the file renaming issue.

I want to spend a bit of time today testing this locally to make sure we generate fresh config and read existing config correctly etc.

Do you think it is worth it to write the migration code for it? Considering it's floonnet/testnet?

@antiochp
Copy link
Member

antiochp commented Oct 7, 2020

Thanks for the review. Addressed the file renaming issue.

I want to spend a bit of time today testing this locally to make sure we generate fresh config and read existing config correctly etc.

Do you think it is worth it to write the migration code for it? Considering it's floonnet/testnet?

No I think its fine if we just need to recreate config etc. I just want to make sure there are no surprises in there.

@antiochp
Copy link
Member

antiochp commented Oct 7, 2020

Regenerated --testnet config file and resyncing now. Everything looks good to me.

antiochp
antiochp approved these changes Oct 7, 2020
View reviewed changes
Copy link
Member

@antiochp antiochp left a comment

Choose a reason for hiding this comment

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

👍

@quentinlesceller
Copy link
Member Author

Okay LGTM

@quentinlesceller quentinlesceller merged commit cf2a652 into mimblewimble:master Oct 7, 2020
@quentinlesceller quentinlesceller deleted the testnet branch October 7, 2020 13:36
@antiochp antiochp mentioned this pull request Nov 26, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antiochp antiochp antiochp approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Consider: rename floonet -> testnet
2 participants
@quentinlesceller @antiochp