Repository: farcaster-project/farcaster-node
Pull request #205 (2021-11-19 23:31:32)

Monero address encoding

codecov-commenter: # [Codecov](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#205](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (3bd5946) into [main](https://codecov.io/gh/farcaster-project/farcaster-node/commit/b884e8afe03ae8f6774fd45f96dda911c56bae0d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (b884e8a) will **decrease** coverage by `0.3%`.
> The diff coverage is `3.9%`.

[![Impacted file tree graph](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205/graphs/tree.svg?width=650&height=150&src=pr&token=ZZ9RnFhgFC&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

```diff
@@           Coverage Diff           @@
##            main    #205     +/-   ##
=======================================
- Coverage   15.5%   15.2%   -0.3%     
=======================================
  Files         27      27             
  Lines       6071    6124     +53     
=======================================
- Hits         941     930     -11     
- Misses      5130    5194     +64     
```


| [Impacted Files](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [src/error.rs](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2Vycm9yLnJz) | `1.8% <0.0%> (-<0.1%)` | :arrow_down: |
| [src/farcasterd/runtime.rs](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2ZhcmNhc3RlcmQvcnVudGltZS5ycw==) | `0.0% <0.0%> (ø)` | |
| [src/rpc/request.rs](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3JwYy9yZXF1ZXN0LnJz) | `14.2% <0.0%> (-0.6%)` | :arrow_down: |
| [src/swapd/runtime.rs](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3N3YXBkL3J1bnRpbWUucnM=) | `0.0% <0.0%> (ø)` | |
| [src/syncerd/monero\_syncer.rs](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3N5bmNlcmQvbW9uZXJvX3N5bmNlci5ycw==) | `0.0% <0.0%> (ø)` | |
| [src/walletd/runtime.rs](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3dhbGxldGQvcnVudGltZS5ycw==) | `0.0% <0.0%> (ø)` | |
| [src/syncerd/types.rs](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3N5bmNlcmQvdHlwZXMucnM=) | `28.2% <5.0%> (-9.0%)` | :arrow_down: |
| [src/syncerd/syncer\_state.rs](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3N5bmNlcmQvc3luY2VyX3N0YXRlLnJz) | `92.8% <100.0%> (ø)` | |
| [src/cli/command.rs](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NsaS9jb21tYW5kLnJz) | `36.1% <0.0%> (-6.6%)` | :arrow_down: |
| [src/lib.rs](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2xpYi5ycw==) | `9.2% <0.0%> (-0.9%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Last update [b884e8a...3bd5946](https://codecov.io/gh/farcaster-project/farcaster-node/pull/205?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).


Patch:
diff --git a/src/error.rs b/src/error.rs
index 7c77aec4..6493342e 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -155,6 +155,12 @@ impl From<bitcoin::consensus::encode::Error> for Error {
     }
 }
 
+impl From<monero::util::address::Error> for Error {
+    fn from(err: monero::util::address::Error) -> Self {
+        Error::Farcaster(err.to_string())
+    }
+}
+
 impl From<electrum_client::Error> for Error {
     fn from(err: electrum_client::Error) -> Self {
         Error::Syncer(SyncerError::Electrum(err))
diff --git a/src/farcasterd/runtime.rs b/src/farcasterd/runtime.rs
index 339748c0..b1a6e24f 100644
--- a/src/farcasterd/runtime.rs
+++ b/src/farcasterd/runtime.rs
@@ -114,7 +114,7 @@ pub struct Runtime {
     taking_swaps: HashMap<ServiceId, (request::InitSwap, Network)>,
     public_offers: HashSet<PublicOffer<BtcXmr>>,
     arb_addrs: HashMap<PublicOfferId, bitcoin::Address>,
-    acc_addrs: HashMap<PublicOfferId, String>,
+    acc_addrs: HashMap<PublicOfferId, monero::Address>,
     consumed_offers: HashSet<PublicOfferId>,
     node_ids: HashSet<PublicKey>, // TODO is it possible? HashMap<SwapId, PublicKey>
     wallet_token: Token,
@@ -658,7 +658,8 @@ impl Runtime {
                         Request::Success(OptionDetails(Some(msg))),
                     ));
                     self.arb_addrs.insert(pub_offer_id, arbitrating_addr);
-                    self.acc_addrs.insert(pub_offer_id, accordant_addr);
+                    self.acc_addrs
+                        .insert(pub_offer_id, monero::Address::from_str(&accordant_addr)?);
                 } else {
                     let msg = "This Public offer was previously registered";
                     warn!("{}", msg.err());
diff --git a/src/rpc/request.rs b/src/rpc/request.rs
index ae7efcfc..d0157226 100644
--- a/src/rpc/request.rs
+++ b/src/rpc/request.rs
@@ -20,12 +20,14 @@ use crate::syncerd::{
 };
 use crate::walletd::NodeSecrets;
 use amplify::{Holder, ToYamlString, Wrapper};
+use farcaster_core::consensus::{Decodable as CoreDecodable, Encodable as CoreEncodable};
 use farcaster_core::syncer::BroadcastTransaction;
 use farcaster_core::{bundle::SignedArbitratingLock, syncer::Abort};
 use internet2::addr::InetSocketAddr;
 use internet2::{CreateUnmarshaller, Payload, Unmarshall, Unmarshaller};
 use lazy_static::lazy_static;
 use lightning_encoding::{strategies::AsStrict, LightningDecode, LightningEncode};
+use monero::consensus::{Decodable as MoneroDecodable, Encodable as MoneroEncodable};
 #[cfg(feature = "serde")]
 use serde_with::{DisplayFromStr, DurationSeconds, Same};
 use std::{collections::BTreeMap, convert::TryInto};
@@ -564,9 +566,38 @@ pub struct NodeInfo {
 #[display("bitcoin_address")]
 pub struct BitcoinAddress(pub SwapId, pub bitcoin::Address);
 
-#[derive(Clone, PartialEq, Eq, Debug, Display, StrictEncode, StrictDecode)]
+impl StrictEncode for MoneroAddress {
+    fn strict_encode<E: ::std::io::Write>(
+        &self,
+        mut e: E,
+    ) -> Result<usize, strict_encoding::Error> {
+        let len = self
+            .0
+            .consensus_encode(&mut e)
+            .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?;
+        use monero::consensus::Encodable;
+        Ok(len
+            + self
+                .1
+                .consensus_encode(&mut e)
+                .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?)
+    }
+}
+
+impl StrictDecode for MoneroAddress {
+    fn strict_decode<D: ::std::io::Read>(mut d: D) -> Result<Self, strict_encoding::Error> {
+        Ok(Self(
+            SwapId::consensus_decode(&mut d)
+                .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?,
+            monero::Address::consensus_decode(&mut d)
+                .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?,
+        ))
+    }
+}
+
+#[derive(Clone, PartialEq, Eq, Debug, Display)]
 #[display("monero_address")]
-pub struct MoneroAddress(pub SwapId, pub String);
+pub struct MoneroAddress(pub SwapId, pub monero::Address);
 
 #[derive(Clone, PartialEq, Eq, Debug, Display, StrictEncode, StrictDecode)]
 #[display("proto_puboffer")]
diff --git a/src/swapd/runtime.rs b/src/swapd/runtime.rs
index 75829ac6..28149ddb 100644
--- a/src/swapd/runtime.rs
+++ b/src/swapd/runtime.rs
@@ -530,7 +530,7 @@ impl SyncerState {
         &mut self,
         view_key: monero::PrivateKey,
         spend_key: monero::PrivateKey,
-        address: String,
+        address: monero::Address,
     ) -> Task {
         let id = self.tasks.new_taskid();
         self.tasks.sweeping_addr = Some(id);
diff --git a/src/syncerd/monero_syncer.rs b/src/syncerd/monero_syncer.rs
index 2850f29e..39f41e12 100644
--- a/src/syncerd/monero_syncer.rs
+++ b/src/syncerd/monero_syncer.rs
@@ -501,7 +501,7 @@ fn sweep_polling(
                 {
                     let mut sweep_address_txs = None;
                     match sweep_address(
-                        monero::Address::from_str(&addendum.address).unwrap(),
+                        addendum.address,
                         addendum.view_key,
                         addendum.spend_key,
                         &network,
diff --git a/src/syncerd/syncer_state.rs b/src/syncerd/syncer_state.rs
index 024b3314..a5dca4dd 100644
--- a/src/syncerd/syncer_state.rs
+++ b/src/syncerd/syncer_state.rs
@@ -890,7 +890,10 @@ async fn syncer_state_sweep_addresses() {
                 "77916d0cd56ed1920aef6ca56d8a41bac915b68e4c46a589e0956e27a7b77404",
             )
             .unwrap(),
-            address: "address".to_string(),
+            address: monero::Address::from_str(
+                "51qzspbPiQ9Z9Wq3hR8HRhPmVcE3URCK8b8A9ypHHzyvhigWTefCapoG1MXVZQQi7B5t4DpJYrHZyaFjHSb5QqLe8YEaBpo"
+            )
+            .unwrap(),
         }),
     };
     let source1 = ServiceId::Syncer(Coin::Monero, Network::Mainnet);
diff --git a/src/syncerd/types.rs b/src/syncerd/types.rs
index b4d10dde..85b3dc23 100644
--- a/src/syncerd/types.rs
+++ b/src/syncerd/types.rs
@@ -1,4 +1,5 @@
-use farcaster_core::consensus::{self, CanonicalBytes, Decodable, Encodable};
+use monero::consensus::Decodable;
+use monero::consensus::Encodable;
 use std::io;
 use strict_encoding::{StrictDecode, StrictEncode};
 
@@ -29,42 +30,37 @@ pub struct XmrAddressAddendum {
     pub from_height: u64,
 }
 
-impl Encodable for XmrAddressAddendum {
-    fn consensus_encode<W: io::Write>(&self, s: &mut W) -> Result<usize, io::Error> {
-        let mut len = self.spend_key.as_canonical_bytes().consensus_encode(s)?;
-        len += self.view_key.as_canonical_bytes().consensus_encode(s)?;
-        Ok(len + self.from_height.consensus_encode(s)?)
-    }
-}
-
-impl Decodable for XmrAddressAddendum {
-    fn consensus_decode<D: io::Read>(d: &mut D) -> Result<Self, consensus::Error> {
-        Ok(Self {
-            spend_key: monero::PublicKey::from_canonical_bytes(
-                Vec::<u8>::consensus_decode(d)?.as_ref(),
-            )?,
-            view_key: monero::PrivateKey::from_canonical_bytes(
-                Vec::<u8>::consensus_decode(d)?.as_ref(),
-            )?,
-            from_height: u64::consensus_decode(d)?,
-        })
-    }
-}
-
 impl StrictEncode for XmrAddressAddendum {
     fn strict_encode<E: ::std::io::Write>(
         &self,
         mut e: E,
     ) -> Result<usize, strict_encoding::Error> {
-        farcaster_core::consensus::Encodable::consensus_encode(self, &mut e)
-            .map_err(strict_encoding::Error::from)
+        let mut len = self
+            .spend_key
+            .consensus_encode(&mut e)
+            .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?;
+        len += self
+            .view_key
+            .consensus_encode(&mut e)
+            .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?;
+        Ok(len
+            + self
+                .from_height
+                .consensus_encode(&mut e)
+                .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?)
     }
 }
 
 impl StrictDecode for XmrAddressAddendum {
     fn strict_decode<D: ::std::io::Read>(mut d: D) -> Result<Self, strict_encoding::Error> {
-        farcaster_core::consensus::Decodable::consensus_decode(&mut d)
-            .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))
+        Ok(Self {
+            spend_key: monero::PublicKey::consensus_decode(&mut d)
+                .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?,
+            view_key: monero::PrivateKey::consensus_decode(&mut d)
+                .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?,
+            from_height: u64::consensus_decode(&mut d)
+                .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?,
+        })
     }
 }
 
@@ -88,29 +84,7 @@ pub enum SweepAddressAddendum {
 pub struct SweepXmrAddress {
     pub spend_key: monero::PrivateKey,
     pub view_key: monero::PrivateKey,
-    pub address: String,
-}
-
-impl Encodable for SweepXmrAddress {
-    fn consensus_encode<W: io::Write>(&self, s: &mut W) -> Result<usize, io::Error> {
-        let mut len = self.spend_key.as_canonical_bytes().consensus_encode(s)?;
-        len += self.view_key.as_canonical_bytes().consensus_encode(s)?;
-        Ok(len + self.address.consensus_encode(s)?)
-    }
-}
-
-impl Decodable for SweepXmrAddress {
-    fn consensus_decode<D: io::Read>(d: &mut D) -> Result<Self, consensus::Error> {
-        Ok(Self {
-            spend_key: monero::PrivateKey::from_canonical_bytes(
-                Vec::<u8>::consensus_decode(d)?.as_ref(),
-            )?,
-            view_key: monero::PrivateKey::from_canonical_bytes(
-                Vec::<u8>::consensus_decode(d)?.as_ref(),
-            )?,
-            address: String::consensus_decode(d)?,
-        })
-    }
+    pub address: monero::Address,
 }
 
 impl StrictEncode for SweepXmrAddress {
@@ -118,15 +92,32 @@ impl StrictEncode for SweepXmrAddress {
         &self,
         mut e: E,
     ) -> Result<usize, strict_encoding::Error> {
-        farcaster_core::consensus::Encodable::consensus_encode(self, &mut e)
-            .map_err(strict_encoding::Error::from)
+        let mut len = self
+            .spend_key
+            .consensus_encode(&mut e)
+            .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?;
+        len += self
+            .view_key
+            .consensus_encode(&mut e)
+            .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?;
+        Ok(len
+            + self
+                .address
+                .consensus_encode(&mut e)
+                .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?)
     }
 }
 
 impl StrictDecode for SweepXmrAddress {
     fn strict_decode<D: ::std::io::Read>(mut d: D) -> Result<Self, strict_encoding::Error> {
-        farcaster_core::consensus::Decodable::consensus_decode(&mut d)
-            .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))
+        Ok(Self {
+            spend_key: monero::PrivateKey::consensus_decode(&mut d)
+                .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?,
+            view_key: monero::PrivateKey::consensus_decode(&mut d)
+                .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?,
+            address: monero::Address::consensus_decode(&mut d)
+                .map_err(|e| strict_encoding::Error::DataIntegrityError(e.to_string()))?,
+        })
     }
 }
 
diff --git a/src/walletd/runtime.rs b/src/walletd/runtime.rs
index 8e4ab388..8935d106 100644
--- a/src/walletd/runtime.rs
+++ b/src/walletd/runtime.rs
@@ -90,7 +90,7 @@ pub struct Runtime {
     wallets: HashMap<SwapId, Wallet>,
     swaps: HashMap<SwapId, Option<Request>>,
     btc_addrs: HashMap<SwapId, bitcoin::Address>,
-    xmr_addrs: HashMap<SwapId, String>,
+    xmr_addrs: HashMap<SwapId, monero::Address>,
 }
 
 pub enum Wallet {
@@ -948,7 +948,8 @@ impl Runtime {
 
                 let swap_id: SwapId = SwapId::random();
                 self.swaps.insert(swap_id, None);
-                self.xmr_addrs.insert(swap_id, internal_address);
+                self.xmr_addrs
+                    .insert(swap_id, monero::Address::from_str(&internal_address)?);
 
                 // since we're takers, we are on the other side of the trade
                 let taker_role = offer.maker_role.other();


Test patch:
diff --git a/tests/functional.rs b/tests/functional.rs
index bee8d1b1..3876c66d 100644
--- a/tests/functional.rs
+++ b/tests/functional.rs
@@ -874,7 +874,7 @@ async fn monero_syncer_sweep_test() {
         spend: spend_key,
     };
     let to_be_sweeped_address = monero::Address::from_keypair(monero::Network::Mainnet, &keypair);
-    let dest_address = "43qHP7gSJJf8HZw1G3ZmpWVyYnbxkKdfta34Qj2nuRENjAsXBtj9JcMWcYMeT3n4NyTZqxhUkKgsTS6P2TNgM6ksM32czSp";
+    let dest_address = monero::Address::from_str("43qHP7gSJJf8HZw1G3ZmpWVyYnbxkKdfta34Qj2nuRENjAsXBtj9JcMWcYMeT3n4NyTZqxhUkKgsTS6P2TNgM6ksM32czSp").unwrap();
     send_monero(&wallet, to_be_sweeped_address, 1000000000000).await;
 
     let task = SyncerdTask {
@@ -884,7 +884,7 @@ async fn monero_syncer_sweep_test() {
             addendum: SweepAddressAddendum::Monero(SweepXmrAddress {
                 spend_key,
                 view_key,
-                address: dest_address.to_string(),
+                address: dest_address,
             }),
         }),
         source: SOURCE2.clone(),