Feature/btc native segwit #41
Conversation
release 2.3.0
…ore into feature/native_segwit
| let path_array: Vec<&str> = account_path.split(";").collect(); | ||
|
|
||
| for (index, path) in path_array.iter().enumerate() { | ||
| let mut address_0_0 = "".to_string(); |
There was a problem hiding this comment.
if 中包含所有 case 的话这边应该可以直接用 let address_0_0: String即可
| let mut address_0_1 = "".to_string(); | ||
| if path.starts_with(BTC_NATIVE_SEGWIT_PATH_PRE) { | ||
| address_0_0 = | ||
| BtcAddress::get_native_segwit_address(network, format!("{}/0/0", path).as_str())?; |
| receive_address = address_0_1; | ||
| enc_xpub = xpub; | ||
| } else { | ||
| main_address = format!("{};{}", main_address, address_0_0); |
There was a problem hiding this comment.
这种拼接方法感觉怪怪的。似乎正常来说先生成一个vec所有元素在添加到这个vec中。最后一次性再使用 join 合并到一起。
在当前的情况来看因为 main_address 第一个是空的。 所以如果只有一个地址的情况下应该会出现 ";addr_1" 这种字符串。我担心后续可能会不小心因为这种情况出现bug
| "NONE" => display_btc_legacy_address(param), | ||
| "P2WPKH" => display_segwit_address(param), | ||
| "BECH32" => display_native_segwit_address(param), | ||
| _ => display_native_segwit_address(param), |
There was a problem hiding this comment.
目前所有seg_wit应该都会由前端送，所以有一种场景是 前端送了一个新的类别。匹配到 _ 不会报错，但实际是错误结果。我建议 _ 的地方直接报错。
| let set_wit = "NONE"; | ||
| let network = network_from_param(¶m.chain_type, ¶m.network, &set_wit).unwrap(); | ||
| address = BtcForkAddress::p2pkh(&network, ¶m.path)?; | ||
| } else { | ||
| let set_wit = "P2WPKH"; |
There was a problem hiding this comment.
最好这些字符串可以定义为常量。防止某些地方不小心写错，但编译器不会报错。
| } | ||
| //add change output | ||
| if self.get_change_amount() > DUST_THRESHOLD { | ||
| let path_temp = format!("{}{}{}", path_str, "1/", change_idx); |
There was a problem hiding this comment.
呃呃，这边可以直接写成 format!("{}1/{}")
| txhash: "983adf9d813a2b8057454cc6f36c6081948af849966f9b9a33e5b653b02f227a".to_string(), | ||
| vout: 0, | ||
| //use local private key sign data | ||
| let mut output_pareper_data = |
There was a problem hiding this comment.
这里是prepare拼错了还是确实要用 pareper?
|
|
||
| data.extend(address_data.iter()); | ||
| if index == self.unspents.len() - 1 { | ||
| sign_apdu_vec.push(BtcApdu::btc_segwit_sign(true, 0x01, data)); |
There was a problem hiding this comment.
这边给 index == self.unspents.len() -1 一个变量名的话里面不用重复这么多代码，而且这个true和false意义更明确一些。目前单看这些代码如果你把true和false调到一下位置的话，其他人也不太容易看出代码是对是错
| }; | ||
| let mut output_serialize_data = serialize(&tx_to_sign); | ||
|
|
||
| output_serialize_data.remove(5); |
There was a problem hiding this comment.
这里看起来怪怪的，只能猜测是删除尾部两个元素。但是感觉后续的各种remove和insert都太过于固话，比如整体序列化的元素少了一个这里面所有步骤都要该。不能先判断出 sign type, input number 01 tag这种之后直接在一个方法拼接出来吗？
| }; | ||
| let sign_result = transaction_req_data.sign_segwit_transaction( | ||
| Network::Testnet, | ||
| &"m/49'/1'/0'/".to_string(), |
There was a problem hiding this comment.
我看有的测试包含最后一个 / 有的不包含，实际代码中大部分拼接path都是 "0/" 这种要求包含前置 / 的。这部分代码中是有处理的是吧？
BTC native segwit transaction feature