-
Notifications
You must be signed in to change notification settings - Fork 152
Port to candidate 1.2 #1911
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
Merged
Merged
Port to candidate 1.2 #1911
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
remoun
approved these changes
May 3, 2022
eranrund
approved these changes
May 3, 2022
dolanbernard
approved these changes
May 3, 2022
iamalwaysuncomfortable
approved these changes
May 3, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see nothing out of order here, approved
this passed a deployment |
Base automatically changed from
port-improvements-to-amount-and-transaction-builder
to
candidate-1.2
May 4, 2022 18:40
There was no real conflict except that in the tests for the new memo builder, the tests had to change the .add_output and .add_change_output APIs are different.
Set Change Subaddress Index to u64::MAX - 1
This conflict was resolved: ``` diff --cc mobilecoind/src/payments.rs index 6424c3f,c3b6f0ed..00000000 --- a/mobilecoind/src/payments.rs +++ b/mobilecoind/src/payments.rs @@@ -867,18 -876,17 +876,32 @@@ impl<T: BlockchainConnection + UserTxCo fog_resolver_factory(&fog_uris).map_err(Error::Fog)? }; ++<<<<<<< HEAD + // TODO: Use RTH memo builder, optionally? + + let fee_amount = Amount::new(fee, token_id); + + // Create tx_builder. + let mut tx_builder = TransactionBuilder::new( + block_version, + fee_amount, + fog_resolver, + EmptyMemoBuilder::default(), + ) + .map_err(|err| Error::TxBuild(format!("Error cretaing TransactionBuilder: {}", err)))?; ++======= + // Create tx_builder. + // TODO (GH #1522): Use RTH memo builder, optionally? + let memo_builder: Box<dyn MemoBuilder + Send + Sync> = + opt_memo_builder.unwrap_or_else(|| Box::new(EmptyMemoBuilder::default())); + + let fee_amount = Amount::new(fee, token_id); + let mut tx_builder = + TransactionBuilder::new_with_box(block_version, fee_amount, fog_resolver, memo_builder) + .map_err(|err| { + Error::TxBuild(format!("Error creating transaction builder: {}", err)) + })?; ++>>>>>>> d991eee... API for generating burn txs in mobilecoind (#1872) // Unzip each vec of tuples into a tuple of vecs. let mut rings_and_proofs: Vec<(Vec<TxOut>, Vec<TxOutMembershipProof>)> = rings ```
There was a very small conflict which I resolved
31341b0
to
c9364d3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR cherry-picks a series of PRs in master branch which landed after Mixed transactions, to the candidate 1.2 branch.
It is staged after the PR that moves pieces of Mixed transactions to candidate 1.2 branch.
The PRs that are picked are:
a26b06f "Initial take on implementing Burn Redemption Memo" (eran)
e4038aa"Update Change Subaddress Index (#1880)" (bernie)
d991eee "API for generating burn txs in mobilecoind (#1872)" (eran)
83dcf52 "fix typo (#1883)" (eran)
3fac7a8 "Fix/slip10errors" #1893 (william)
If this passes tests and CD then I propose that we merge both PRs and then rename
candidate-1.2
torelease-1.2
or something like this.