Upgrade to handle v5 transactions #28
Conversation
ledger-zcash/src/app.rs
Outdated
| let hex_tx_version = match tx_version { | ||
| TxVersion::Zip225 => 0x05, | ||
| TxVersion::Sapling => 0x04, | ||
| _ => 0x04, //todo: what should the default be? |
There was a problem hiding this comment.
Due to it being an enum it represents all the possible values, but the app doesn't support it all, so you should return an error instead of having this default
zcash-hsmbuilder/src/txbuilder.rs
Outdated
| progress_notifier.as_ref().map(|tx| tx.send(progress)); | ||
| if let Some(sender) = progress_notifier.as_ref() { | ||
| // If the send fails, we should ignore the error, not crash. | ||
| sender.send(Progress::new(progress, None)).unwrap_or(()); |
There was a problem hiding this comment.
if you want to ignore the error just let _ = ... instead, it's the conventional way to ignore erros
zcash-hsmbuilder/Cargo.toml
Outdated
| @@ -41,7 +41,8 @@ rand_core = "0.6" | |||
| ripemd = "0.1" | |||
| secp256k1 = { version = "0.21" } | |||
| sha2 = "0.9" | |||
|
|
|||
| log4rs = "1" | |||
There was a problem hiding this comment.
unused - but should be used for binaries, not libraries (in that case log should be used)
zcash-hsmbuilder/src/txbuilder.rs
Outdated
| @@ -137,7 +143,7 @@ where | |||
| pub fn transaction_data(&self) -> Option<TransactionData<A>> { | |||
| self.cached_branchid.map(|consensus_branch_id| { | |||
| TransactionData::from_parts( | |||
| transaction::TxVersion::Sapling, | |||
| self.cached_tx_version.expect("No tx version provided"), | |||
There was a problem hiding this comment.
avoid the panic here, rather return None if cached_tx is none, just like we would with cached_branchid
There was a problem hiding this comment.
Should be able to do that with zip https://doc.rust-lang.org/std/option/enum.Option.html#method.zip
zcash-hsmbuilder/src/txbuilder.rs
Outdated
| progress_notifier.as_ref().map(|tx| tx.send(progress)); | ||
| if let Some(sender) = progress_notifier.as_ref() { | ||
| // If the send fails, we should ignore the error, not crash. | ||
| sender.send(Progress::new(progress, None)).unwrap_or(()); |
zcash-hsmbuilder/src/txbuilder.rs
Outdated
| @@ -832,7 +880,7 @@ impl<P: consensus::Parameters, R: RngCore + CryptoRng> | |||
| fn transaction_data_authorized(&self) -> Option<TransactionData<transaction::Authorized>> { | |||
| self.cached_branchid.map(|consensus_branch_id| { | |||
| TransactionData::from_parts( | |||
| transaction::TxVersion::Sapling, | |||
| self.cached_tx_version.expect("No tx version provided"), | |||
There was a problem hiding this comment.
same as above, zip and return None if it's missing
refactor: pad sapling outputs within change setup method to avoid extra fees
ghost
self-requested a review
|
Also includes ZIP-0317 |
🔗 zboto Link