feat(api-redis): Make AccountDetails.ilp_address optional

If an account is inserted without an , then 2 things can happen:
1. If their username is the same as the store's last segment, then their address is the same as the store. This allows to insert ourselves' account without having to specify our address
2. Otherwise, their address is created by appending their username to the store's ilp address

Also adjust redis tests for new functionality. When the redis store is
launched, it checks if the address is set and uses that, otherwise uses
the default store address

crates/interledger-api/src/lib.rs

@@ -3,7 +3,7 @@ use futures::Future;
 use interledger_http::{HttpAccount, HttpServer as IlpOverHttpServer, HttpStore};
 use interledger_packet::Address;
 use interledger_router::RouterStore;
-use interledger_service::{Account, IncomingService, Username};
+use interledger_service::{Account, AddressStore, IncomingService, Username};
 use interledger_service_util::{BalanceStore, ExchangeRateStore};
 use interledger_settlement::{SettlementAccount, SettlementStore};
 use interledger_stream::StreamNotificationsStore;
@@ -18,7 +18,7 @@ mod routes;
 
 pub(crate) mod http_retry;
 
-pub trait NodeStore: Clone + Send + Sync + 'static {
+pub trait NodeStore: AddressStore + Clone + Send + Sync + 'static {
     type Account: Account;
 
     fn insert_account(
@@ -79,9 +79,9 @@ pub struct AccountSettings {
 }
 
 /// The Account type for the RedisStore.
-#[derive(Debug, Clone, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct AccountDetails {
-    pub ilp_address: Address,
+    pub configured_ilp_address: Option<Address>,
     pub username: Username,
     pub asset_code: String,
     pub asset_scale: u8,

crates/interledger-store-redis/src/account.rs

@@ -21,7 +21,6 @@ use serde::{Deserialize, Serialize};
 use std::fmt::Display;
 use std::{
     collections::HashMap,
-    convert::TryFrom,
     str::{self, FromStr},
 };
 use uuid::{parser::ParseError, Uuid};
@@ -61,6 +60,7 @@ pub struct Account {
     pub(crate) amount_per_minute_limit: Option<u64>,
     pub(crate) settlement_engine_url: Option<Url>,
 }
+
 fn address_to_string<S>(address: &Address, serializer: S) -> Result<S::Ok, S::Error>
 where
     S: Serializer,
@@ -79,7 +79,32 @@ where
 }
 
 impl Account {
-    pub fn try_from(id: AccountId, details: AccountDetails) -> Result<Account, ()> {
+    pub fn try_from(
+        id: AccountId,
+        details: AccountDetails,
+        parent_ilp_address: Address,
+    ) -> Result<Account, ()> {
+        let ilp_address = match details.configured_ilp_address {
+            Some(a) => a,
+            None => {
+                // if parent address ends with username, do not suffix
+                if details.username.to_string()
+                    == parent_ilp_address.segments().rev().next().unwrap()
+                {
+                    parent_ilp_address
+                } else {
+                    parent_ilp_address
+                        .with_suffix(details.username.as_bytes())
+                        .map_err(|_| {
+                            error!(
+                                "Could not append username {} to address {}",
+                                details.username, parent_ilp_address
+                            )
+                        })?
+                }
+            }
+        };
+
         let http_endpoint = if let Some(ref url) = details.http_endpoint {
             Some(Url::parse(url).map_err(|err| error!("Invalid URL: {:?}", err))?)
         } else {
@@ -126,9 +151,7 @@ impl Account {
         Ok(Account {
             id,
             username: details.username,
-            ilp_address: Address::try_from(details.ilp_address.as_ref()).map_err(|err| {
-                error!("Invalid ILP Address when creating Redis account: {:?}", err)
-            })?,
+            ilp_address,
             asset_code: details.asset_code.to_uppercase(),
             asset_scale: details.asset_scale,
             max_packet_amount: details.max_packet_amount,
@@ -546,7 +569,7 @@ mod redis_account {
 
     lazy_static! {
         static ref ACCOUNT_DETAILS: AccountDetails = AccountDetails {
-            ilp_address: Address::from_str("example.alice").unwrap(),
+            configured_ilp_address: Some(Address::from_str("example.alice").unwrap()),
             username: Username::from_str("alice").unwrap(),
             asset_scale: 6,
             asset_code: "XYZ".to_string(),
@@ -571,7 +594,12 @@ mod redis_account {
     #[test]
     fn from_account_details() {
         let id = AccountId::new();
-        let account = Account::try_from(id, ACCOUNT_DETAILS.clone()).unwrap();
+        let account = Account::try_from(
+            id,
+            ACCOUNT_DETAILS.clone(),
+            Address::from_str("local.host").unwrap(),
+        )
+        .unwrap();
         assert_eq!(account.id(), id);
         assert_eq!(
             account.get_http_auth_token().unwrap(),

crates/interledger-store-redis/src/store.rs

@@ -30,7 +30,7 @@ use super::account::AccountId;
 use http::StatusCode;
 use interledger_api::{AccountDetails, AccountSettings, NodeStore};
 use interledger_btp::BtpStore;
-use interledger_ccp::{CcpRoutingAccount, RouteManagerStore};
+use interledger_ccp::{CcpRoutingAccount, RouteManagerStore, RoutingRelation};
 use interledger_http::HttpStore;
 use interledger_packet::Address;
 use interledger_router::RouterStore;
@@ -370,16 +370,23 @@ impl RedisStore {
         let connection = self.connection.clone();
         let routing_table = self.routes.clone();
         let encryption_key = self.encryption_key.clone();
-
         let id = AccountId::new();
-        debug!("Generated account: {}", id);
+        let ilp_address = self.get_ilp_address();
+        debug!(
+            "Generated account id for {}: {}",
+            account.username.clone(),
+            id
+        );
         Box::new(
-            result(Account::try_from(id, account))
+            result(Account::try_from(id, account, ilp_address))
                 .and_then(move |account| {
                     // Check that there isn't already an account with values that MUST be unique
                     let mut pipe = redis::pipe();
                     pipe.exists(accounts_key(account.id));
                     pipe.hexists("usernames", account.username().as_ref());
+                    if account.routing_relation == RoutingRelation::Parent {
+                        pipe.exists(PARENT_ILP_KEY);
+                    }
 
                     pipe.query_async(connection.as_ref().clone())
                         .map_err(|err| {
@@ -429,6 +436,9 @@ impl RedisStore {
                     pipe.hset(ROUTES_KEY, account.ilp_address.to_bytes().to_vec(), account.id)
                         .ignore();
 
+                    // The parent account settings are done via the API. We just
+                    // had to check for the existence of a parent
+
                     pipe.query_async(connection)
                         .map_err(|err| error!("Error inserting account into DB: {:?}", err))
                         .and_then(move |(connection, _ret): (SharedConnection, Value)| {
@@ -450,17 +460,22 @@ impl RedisStore {
         let connection = self.connection.clone();
         let routing_table = self.routes.clone();
         let encryption_key = self.encryption_key.clone();
+        let ilp_address = self.get_ilp_address().clone();
 
         Box::new(
             // Check to make sure an account with this ID already exists
             redis::cmd("EXISTS")
                 .arg(accounts_key(id))
-                // TODO this needs to be atomic with the insertions later, waiting on #186
+                // TODO this needs to be atomic with the insertions later,
+                // waiting on #186
+                // TODO: Do not allow this update to happen if
+                // AccountDetails.RoutingRelation == Parent and parent is
+                // already set
                 .query_async(connection.as_ref().clone())
                 .map_err(|err| error!("Error checking whether ID exists: {:?}", err))
                 .and_then(move |(connection, result): (SharedConnection, bool)| {
                     if result {
-                        Account::try_from(id, account)
+                        Account::try_from(id, account, ilp_address)
                             .and_then(move |account| Ok((connection, account)))
                     } else {
                         warn!(
@@ -611,8 +626,7 @@ impl RedisStore {
     ) -> Box<dyn Future<Item = Account, Error = ()> + Send> {
         let connection = self.connection.as_ref().clone();
         let routing_table = self.routes.clone();
-
-        Box::new(self.redis_get_account(id).and_then(|account| {
+        Box::new(self.redis_get_account(id).and_then(move |account| {
             let mut pipe = redis::pipe();
             pipe.atomic();
 

crates/interledger-store-redis/tests/accounts_test.rs

@@ -2,16 +2,60 @@ mod common;
 
 use common::*;
 
+use futures::future::{result, Either};
 use interledger_api::{AccountSettings, NodeStore};
 use interledger_btp::{BtpAccount, BtpStore};
 use interledger_http::{HttpAccount, HttpStore};
 use interledger_packet::Address;
 use interledger_service::Account as AccountTrait;
-use interledger_service::{AccountStore, Username};
+use interledger_service::{AccountStore, AddressStore, Username};
 use interledger_service_util::BalanceStore;
 use interledger_store_redis::AccountId;
+use log::{debug, error};
+use redis::Client;
 use std::str::FromStr;
 
+#[test]
+fn picks_up_parent_during_initialization() {
+    let context = TestContext::new();
+    block_on(
+        result(Client::open(context.get_client_connection_info()))
+            .map_err(|err| error!("Error creating Redis client: {:?}", err))
+            .and_then(|client| {
+                debug!("Connected to redis: {:?}", client);
+                client
+                    .get_shared_async_connection()
+                    .map_err(|err| error!("Error connecting to Redis: {:?}", err))
+            })
+            .and_then(move |connection| {
+                // we set a parent that was already configured via perhaps a
+                // previous account insertion. that means that when we connect
+                // to the store we will always get the configured parent (if
+                // there was one))
+                redis::cmd("SET")
+                    .arg("parent_node_account_address")
+                    .arg("example.bob.node")
+                    .query_async(connection)
+                    .map_err(|err| panic!(err))
+                    .and_then(move |(_, _): (_, redis::Value)| {
+                        RedisStoreBuilder::new(context.get_client_connection_info(), [0; 32])
+                            .connect()
+                            .and_then(move |store| {
+                                // the store's ilp address is the store's
+                                // username appended to the parent's address
+                                assert_eq!(
+                                    *store.ilp_address.read(),
+                                    Address::from_str("example.bob.node").unwrap()
+                                );
+                                let _ = context;
+                                Ok(())
+                            })
+                    })
+            }),
+    )
+    .unwrap();
+}
+
 #[test]
 fn insert_accounts() {
     block_on(test_store().and_then(|(store, context, _accs)| {
@@ -20,7 +64,7 @@ fn insert_accounts() {
             .and_then(move |account| {
                 assert_eq!(
                     *account.ilp_address(),
-                    Address::from_str("example.charlie").unwrap()
+                    Address::from_str("example.alice.user1.charlie").unwrap()
                 );
                 let _ = context;
                 Ok(())
@@ -29,6 +73,41 @@ fn insert_accounts() {
     .unwrap();
 }
 
+#[test]
+fn only_one_parent_allowed() {
+    let mut acc = ACCOUNT_DETAILS_2.clone();
+    acc.routing_relation = Some("Parent".to_owned());
+    acc.username = Username::from_str("another_name").unwrap();
+    acc.configured_ilp_address = Some(Address::from_str("example.another_name").unwrap());
+    block_on(test_store().and_then(|(store, context, accs)| {
+        // check that the store's ilp address is <parent address>.username
+        // assert_eq!(store.ilp
+        store.insert_account(acc.clone()).then(move |res| {
+            // This should fail
+            assert!(res.is_err());
+            // remove account 0 and try again -- must also clear the ILP address
+            // since this was a parent account
+            futures::future::join_all(vec![
+                Either::A(store.delete_account(accs[0].id()).and_then(|_| Ok(()))),
+                Either::B(store.clear_ilp_address()),
+            ])
+            .and_then(move |_| {
+                store.insert_account(acc).and_then(move |account| {
+                    assert_eq!(
+                        *account.ilp_address(),
+                        Address::from_str("example.another_name").unwrap()
+                    );
+
+                    // the store's ilp address should be <new parent address>.username
+                    let _ = context;
+                    Ok(())
+                })
+            })
+        })
+    }))
+    .unwrap();
+}
+
 #[test]
 fn delete_accounts() {
     block_on(test_store().and_then(|(store, context, _accs)| {

crates/interledger-store-redis/tests/balances_test.rs

@@ -8,8 +8,8 @@ use interledger_service::{AccountStore, Username};
 use interledger_service_util::BalanceStore;
 use std::str::FromStr;
 
-use interledger_service::Account as AccountTrait;
-use interledger_store_redis::{Account, AccountId};
+use interledger_service::{Account as AccountTrait, AddressStore};
+use interledger_store_redis::AccountId;
 
 #[test]
 fn get_balance() {
@@ -28,8 +28,12 @@ fn get_balance() {
                     .query_async(connection)
                     .map_err(|err| panic!(err))
                     .and_then(move |(_, _): (_, redis::Value)| {
-                        let account =
-                            Account::try_from(account_id, ACCOUNT_DETAILS_0.clone()).unwrap();
+                        let account = Account::try_from(
+                            account_id,
+                            ACCOUNT_DETAILS_0.clone(),
+                            store.get_ilp_address(),
+                        )
+                        .unwrap();
                         store.get_balance(account).and_then(move |balance| {
                             assert_eq!(balance, 1000);
                             let _ = context;
@@ -95,7 +99,7 @@ fn process_fulfill_no_settle_to() {
     let acc = {
         let mut acc = ACCOUNT_DETAILS_1.clone();
         acc.username = Username::from_str("charlie").unwrap();
-        acc.ilp_address = Address::from_str("example.charlie").unwrap();
+        acc.configured_ilp_address = Some(Address::from_str("example.charlie").unwrap());
         acc.http_incoming_token = None;
         acc.http_outgoing_token = None;
         acc.btp_incoming_token = None;
@@ -131,7 +135,7 @@ fn process_fulfill_settle_to_over_threshold() {
     let acc = {
         let mut acc = ACCOUNT_DETAILS_1.clone();
         acc.username = Username::from_str("charlie").unwrap();
-        acc.ilp_address = Address::from_str("example.b").unwrap();
+        acc.configured_ilp_address = Some(Address::from_str("example.b").unwrap());
         acc.settle_to = Some(101);
         acc.settle_threshold = Some(100);
         acc.http_incoming_token = None;
@@ -168,7 +172,7 @@ fn process_fulfill_ok() {
     let acc = {
         let mut acc = ACCOUNT_DETAILS_1.clone();
         acc.username = Username::from_str("charlie").unwrap();
-        acc.ilp_address = Address::from_str("example.c").unwrap();
+        acc.configured_ilp_address = Some(Address::from_str("example.c").unwrap());
         acc.settle_to = Some(0);
         acc.settle_threshold = Some(100);
         acc.http_incoming_token = None;

crates/interledger-store-redis/tests/btp_test.rs

@@ -15,7 +15,7 @@ fn gets_account_from_btp_auth() {
             .and_then(move |account| {
                 assert_eq!(
                     *account.ilp_address(),
-                    Address::from_str("example.bob").unwrap()
+                    Address::from_str("example.alice.user1.bob").unwrap()
                 );
                 let _ = context;
                 Ok(())

crates/interledger-store-redis/tests/common/fixtures.rs

@@ -7,7 +7,7 @@ use std::str::FromStr;
 lazy_static! {
     // We are dylan starting a connection with all these accounts
     pub static ref ACCOUNT_DETAILS_0: AccountDetails = AccountDetails {
-        ilp_address: Address::from_str("example.alice").unwrap(),
+        configured_ilp_address: Some(Address::from_str("example.alice").unwrap()),
         username: Username::from_str("alice").unwrap(),
         asset_scale: 6,
         asset_code: "XYZ".to_string(),
@@ -27,7 +27,7 @@ lazy_static! {
         settlement_engine_url: None,
     };
     pub static ref ACCOUNT_DETAILS_1: AccountDetails = AccountDetails {
-        ilp_address: Address::from_str("example.bob").unwrap(),
+        configured_ilp_address: None,
         username: Username::from_str("bob").unwrap(),
         asset_scale: 9,
         asset_code: "ABC".to_string(),
@@ -48,7 +48,7 @@ lazy_static! {
         settlement_engine_url: None,
     };
     pub static ref ACCOUNT_DETAILS_2: AccountDetails = AccountDetails {
-        ilp_address: Address::from_str("example.charlie").unwrap(),
+        configured_ilp_address: None,
         username: Username::from_str("charlie").unwrap(),
         asset_scale: 9,
         asset_code: "XRP".to_string(),