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.

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

3961 migration address #3967

Merged
merged 2 commits into from Apr 29, 2019

Conversation

@Eirik0
Copy link
Contributor

commented Apr 23, 2019

Closes #3961

@Eirik0

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

This includes the work from #3888 (currently 5 commits) and will be rebased after that is merged.

@Eirik0 Eirik0 self-assigned this Apr 23, 2019

@Eirik0 Eirik0 added this to Needs Prioritization in Arborist Team via automation Apr 23, 2019

@Eirik0 Eirik0 moved this from Needs Prioritization to Arborist Product Priorities in Arborist Team Apr 23, 2019

@Eirik0 Eirik0 moved this from Arborist Product Priorities to In Review in Arborist Team Apr 23, 2019

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Apr 24, 2019

☔️ The latest upstream changes (presumably #3964) made this pull request unmergeable. Please resolve the merge conflicts.

@Eirik0 Eirik0 force-pushed the Eirik0:3961-migration-address branch from 1d566db to e673430 Apr 24, 2019

@mms710 mms710 requested a review from ebfull Apr 25, 2019

@Eirik0 Eirik0 force-pushed the Eirik0:3961-migration-address branch from e673430 to 1943c36 Apr 26, 2019

@Eirik0

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2019

Force pushed with additional work from #3888.

@Eirik0 Eirik0 requested review from str4d, daira and mdr0id Apr 26, 2019

@Eirik0 Eirik0 referenced this pull request Apr 26, 2019

Merged

3938 migration status #3973

@Eirik0 Eirik0 force-pushed the Eirik0:3961-migration-address branch from 1943c36 to 1c1cd34 Apr 28, 2019

@Eirik0

This comment has been minimized.

Copy link
Contributor Author

commented Apr 28, 2019

Rebased on to master.

@daira

daira approved these changes Apr 29, 2019

Copy link
Contributor

left a comment

ACK, but this has no tests.

@LarryRuane LarryRuane requested review from LarryRuane and daira Apr 29, 2019

if (boost::get<libzcash::SaplingPaymentAddress>(&address) == nullptr) {
return InitError(_("-migrationdestaddress must be a valid Sapling address."));
}
}

This comment has been minimized.

Copy link
@str4d

str4d Apr 29, 2019

Contributor

Move this into the second ENABLE_WALLET section of Step 3.

@@ -1671,6 +1681,9 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
}
}

// Set sapling migration status
pwalletMain->fSaplingMigrationEnabled = GetBoolArg("-migration", false);

This comment has been minimized.

Copy link
@str4d

str4d Apr 29, 2019

Contributor

Add this option to the help text.

Add migration options to conf file
Co-authored-by: Simon <simon@bitcartel.com>

@Eirik0 Eirik0 force-pushed the Eirik0:3961-migration-address branch from 1c1cd34 to 8ffd63a Apr 29, 2019

@Eirik0

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

@str4d force pushed with your suggested changes.

@Eirik0 Eirik0 requested a review from str4d Apr 29, 2019

@str4d

str4d approved these changes Apr 29, 2019

Copy link
Contributor

left a comment

utACK modulo one nit, and there must be a test either here or in #3973.

Show resolved Hide resolved src/init.cpp Outdated
remove extra hyphen
Co-Authored-By: Eirik0 <eirik@z.cash>
@Eirik0

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

I added tests for this as part of #3973. It's not ideal to have the disconnect, but it just seemed easier to test once we had the call to get the migration status.

@LarryRuane
Copy link
Contributor

left a comment

utACK (I agree, test is needed, also doc, I did compile)

@Eirik0

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

@zkbot r+

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Apr 29, 2019

📌 Commit b9c7f27 has been approved by Eirik0

@Eirik0 Eirik0 moved this from In Review to Merge Queue in Arborist Team Apr 29, 2019

zkbot added a commit that referenced this pull request Apr 29, 2019

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Apr 29, 2019

⌛️ Testing commit b9c7f27 with merge f283f24...

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Apr 29, 2019

☀️ Test successful - pr-merge
Approved by: Eirik0
Pushing f283f24 to master...

@zkbot zkbot merged commit b9c7f27 into zcash:master Apr 29, 2019

1 check passed

homu Test successful
Details

Arborist Team automation moved this from Merge Queue to Released (Merged in Master) Apr 29, 2019

@mms710 mms710 added this to the v2.0.5 milestone May 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.