From ba541678efb58321d84bfb4f71d72afb410364f4 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Mon, 5 Aug 2019 13:11:55 +0300 Subject: [PATCH 01/22] fix(settlement-client): do not convert scale The scale conversion will be performed on the engine. We must send our asset scale to the engine so that it knows how to handle the number we sent it. --- crates/interledger-settlement/src/client.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/interledger-settlement/src/client.rs b/crates/interledger-settlement/src/client.rs index 037a9441e..fadea1d3f 100644 --- a/crates/interledger-settlement/src/client.rs +++ b/crates/interledger-settlement/src/client.rs @@ -1,4 +1,4 @@ -use super::{Convert, ConvertDetails, Quantity, SettlementAccount}; +use super::{Quantity, SettlementAccount}; use futures::{ future::{err, Either}, Future, @@ -28,10 +28,6 @@ impl SettlementClient { ) -> impl Future { if let Some(settlement_engine) = account.settlement_engine_details() { let mut settlement_engine_url = settlement_engine.url; - let amount = amount.normalize_scale(ConvertDetails { - from: account.asset_scale(), - to: settlement_engine.asset_scale, - }); settlement_engine_url .path_segments_mut() .expect("Invalid settlement engine URL") @@ -43,11 +39,10 @@ impl SettlementClient { amount, settlement_engine_url ); let settlement_engine_url_clone = settlement_engine_url.clone(); - let asset_scale = settlement_engine.asset_scale; let idempotency_uuid = Uuid::new_v4().to_hyphenated().to_string(); return Either::A(self.http_client.post(settlement_engine_url.clone()) .header("Idempotency-Key", idempotency_uuid) - .json(&json!(Quantity::new(amount, asset_scale))) + .json(&json!(Quantity::new(amount, account.asset_scale()))) .send() .map_err(move |err| error!("Error sending settlement command to settlement engine {}: {:?}", settlement_engine_url, err)) .and_then(move |response| { From 5e576ab72019bfb6fe935d09cee128a6a6607495 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Mon, 5 Aug 2019 13:12:40 +0300 Subject: [PATCH 02/22] feat(settlement): Implement asset scale conversion for u256 --- crates/interledger-settlement/src/lib.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/interledger-settlement/src/lib.rs b/crates/interledger-settlement/src/lib.rs index b48a19725..1f71202c4 100644 --- a/crates/interledger-settlement/src/lib.rs +++ b/crates/interledger-settlement/src/lib.rs @@ -19,11 +19,13 @@ mod fixtures; mod message_service; #[cfg(test)] mod test_helpers; +use bigint::uint::U256; pub use api::SettlementApi; pub use client::SettlementClient; pub use message_service::SettlementMessageService; +use std::ops::Div; lazy_static! { pub static ref SE_ILP_ADDRESS: Address = Address::from_str("peer.settle").unwrap(); } @@ -132,6 +134,19 @@ impl Convert for f64 { } } +impl Convert for U256 { + fn normalize_scale(&self, details: ConvertDetails) -> U256 { + let from_scale = details.from; + let to_scale = details.to; + if from_scale >= to_scale { + self.overflowing_mul(U256::from(10u64.pow(u32::from(from_scale - to_scale)))) + .0 + } else { + self.div(U256::from(10u64.pow(u32::from(to_scale - from_scale)))) + } + } +} + #[cfg(test)] mod tests { /// Tests for the asset conversion From a04832aadb1a32ef0c0e3fe47a902bcd45122e7f Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Mon, 5 Aug 2019 13:15:27 +0300 Subject: [PATCH 03/22] feat(settlement): Normalize the amount properly when being notified by the engine --- crates/interledger-settlement/Cargo.toml | 1 + crates/interledger-settlement/src/api.rs | 38 ++++++++++++++++-------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/crates/interledger-settlement/Cargo.toml b/crates/interledger-settlement/Cargo.toml index b08d90a84..f806ea628 100644 --- a/crates/interledger-settlement/Cargo.toml +++ b/crates/interledger-settlement/Cargo.toml @@ -26,6 +26,7 @@ rand = "0.6.5" ring = "0.14.6" tokio-retry = "0.2.0" tokio = "0.1.20" +bigint = "4.4.1" [dev-dependencies] parking_lot = "0.8.0" diff --git a/crates/interledger-settlement/src/api.rs b/crates/interledger-settlement/src/api.rs index 67fcf096f..394268b09 100644 --- a/crates/interledger-settlement/src/api.rs +++ b/crates/interledger-settlement/src/api.rs @@ -7,6 +7,7 @@ use futures::{ future::{err, ok, result, Either}, Future, }; +use bigint::uint::U256 as BigU256; use hyper::{Response, StatusCode}; use interledger_ildcp::IldcpAccount; use interledger_packet::PrepareBuilder; @@ -139,6 +140,8 @@ impl_web! { fn do_receive_settlement(&self, account_id: String, body: Quantity, idempotency_key: Option) -> Box + Send> { let store = self.store.clone(); + let amount = body.amount; + let engine_scale = body.scale; Box::new(result(A::AccountId::from_str(&account_id) .map_err(move |_err| { let error_msg = format!("Unable to parse account id: {}", account_id); @@ -157,29 +160,38 @@ impl_web! { }}) .and_then(move |accounts| { let account = &accounts[0]; - if let Some(settlement_engine) = account.settlement_engine_details() { - Ok((account.clone(), settlement_engine)) + if account.settlement_engine_details().is_some() { + Ok(account.clone()) } else { let error_msg = format!("Account {} does not have settlement engine details configured. Cannot handle incoming settlement", account.id()); error!("{}", error_msg); Err((StatusCode::from_u16(404).unwrap(), error_msg)) } }) - .and_then(move |(account, settlement_engine)| { - let account_id = account.id(); - let amount: u64 = body.amount.parse().unwrap(); - let amount = amount.normalize_scale(ConvertDetails { - // We have account, engine and request asset scale. - // Which one is not required? Still not clear from all the spec discussions. - from: account.asset_scale(), - to: settlement_engine.asset_scale - }); - store.update_balance_for_incoming_settlement(account_id, amount, idempotency_key) + .and_then(move |account| { + result(BigU256::from_dec_str(&amount)) .map_err(move |_| { - let error_msg = format!("Error updating balance of account: {} for incoming settlement of amount: {}", account_id, amount); + let error_msg = format!("Could not convert amount: {:?}", amount); error!("{}", error_msg); (StatusCode::from_u16(500).unwrap(), error_msg) }) + .and_then(move |amount| { + let account_id = account.id(); + let amount = amount.normalize_scale(ConvertDetails { + // scale it down so that it fits in the connector + from: account.asset_scale(), + to: engine_scale, + }); + // after downscaling it, hopefully we can convert to u64 without + // loss of precision + let amount = amount.low_u64(); + store.update_balance_for_incoming_settlement(account_id, amount, idempotency_key) + .map_err(move |_| { + let error_msg = format!("Error updating balance of account: {} for incoming settlement of amount: {}", account_id, amount); + error!("{}", error_msg); + (StatusCode::from_u16(500).unwrap(), error_msg) + }) + }) }).and_then(move |_| { let ret = Bytes::from("Success"); Ok((StatusCode::OK, ret)) From 4fc57a2067f654b93a3b5f42dfada6a13f0bd9bc Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Mon, 5 Aug 2019 13:19:39 +0300 Subject: [PATCH 04/22] fix(eth-se): Scale the amount to settle for based on the body and engine scale If we have configured the engine with asset scale 18, and the connector sends a body with scale 9 and amount 1, we should settle for 1e9 --- .../interledger-settlement-engines/Cargo.toml | 3 ++- .../src/engines/ethereum_ledger/eth_engine.rs | 24 +++++++++++++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/crates/interledger-settlement-engines/Cargo.toml b/crates/interledger-settlement-engines/Cargo.toml index 85cf5d6fe..0882ecac5 100644 --- a/crates/interledger-settlement-engines/Cargo.toml +++ b/crates/interledger-settlement-engines/Cargo.toml @@ -34,6 +34,7 @@ http = "0.1.17" clap = "2.32.0" clarity = { git = "https://github.com/gakonst/clarity" } sha3 = "0.8.2" +bigint = "4.4.1" [dev-dependencies] lazy_static = "1.3" @@ -43,4 +44,4 @@ net2 = "0.2.33" os_type = "2.2.0" rand = "0.7.0" interledger = { path = "../interledger", version = "0.4.0" } -interledger-packet = { path = "../interledger-packet", version = "0.2.1" } \ No newline at end of file +interledger-packet = { path = "../interledger-packet", version = "0.2.1" } diff --git a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs index a1864c7e5..dcfb0df5a 100644 --- a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs +++ b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs @@ -6,6 +6,7 @@ use sha3::{Digest, Keccak256 as Sha3}; use std::collections::HashMap; use std::iter::FromIterator; +use bigint::uint::U256 as BigU256; use ethereum_tx_sign::web3::{ api::Web3, futures::future::{err, join_all, ok, result, Either, Future}, @@ -21,6 +22,7 @@ use reqwest::r#async::{Client, Response as HttpResponse}; use ring::{digest, hmac}; use serde::{Deserialize, Serialize}; use serde_json::json; +use std::mem; use std::{ marker::PhantomData, str::FromStr, @@ -35,7 +37,7 @@ use uuid::Uuid; use crate::stores::redis_ethereum_ledger::*; use crate::{ApiResponse, CreateAccount, SettlementEngine, SettlementEngineApi}; -use interledger_settlement::Quantity; +use interledger_settlement::{Convert, ConvertDetails, Quantity}; const MAX_RETRIES: usize = 10; const ETH_CREATE_ACCOUNT_PREFIX: &[u8] = b"ilp-ethl-create-account-message"; @@ -480,7 +482,7 @@ where ) -> impl Future { let mut url = self.connector_url.clone(); let account_id_clone = account_id.clone(); - let asset_scale = self.asset_scale; + let engine_scale = self.asset_scale; url.path_segments_mut() .expect("Invalid connector URL") .push("accounts") @@ -493,7 +495,7 @@ where client .post(url.clone()) .header("Idempotency-Key", tx_hash.to_string()) - .json(&json!({ "amount": amount.to_string(), "scale" : asset_scale })) + .json(&json!({ "amount": amount.to_string(), "scale" : engine_scale })) .send() .map_err(move |err| { error!( @@ -757,13 +759,27 @@ where body: Quantity, ) -> Box + Send> { let self_clone = self.clone(); + let engine_scale = self.asset_scale; Box::new( - result(U256::from_dec_str(&body.amount).map_err(move |err| { + result(BigU256::from_dec_str(&body.amount).map_err(move |err| { let error_msg = format!("Error converting to U256 {:?}", err); error!("{:?}", error_msg); (StatusCode::from_u16(400).unwrap(), error_msg) })) .and_then(move |amount| { + // If we receive a Quantity { amount: "1", scale: 9}, + // we must normalize it to our engine's scale (typically 18 + // for ETH/ERC20). + // Slightly counter-intuitive that `body.scale` is set as `to`. + let amount = amount.normalize_scale(ConvertDetails { + from: engine_scale, + to: body.scale, + }); + // Typecast to use the Convert trait implemented for + // bigint::uint::U256 while we're using ethereum_types::U256 + // from rust-web3 which just re-exports it. Should be safe. + let amount: U256 = unsafe { mem::transmute(amount) }; + self_clone .load_account(account_id) .map_err(move |err| { From f02937d5e75c5c63ff00ce85870238707c99e4d0 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Mon, 5 Aug 2019 13:57:32 +0300 Subject: [PATCH 05/22] improve tester --- .../tests/eth_ledger_settlement.rs | 16 ++++++++-------- .../tests/eth_xrp_interoperable.rs | 12 ++++++------ .../tests/test_helpers.rs | 15 ++++++++++++--- .../tests/xrp_ledger_settlement.rs | 16 ++++++++-------- 4 files changed, 34 insertions(+), 25 deletions(-) diff --git a/crates/interledger-settlement-engines/tests/eth_ledger_settlement.rs b/crates/interledger-settlement-engines/tests/eth_ledger_settlement.rs index 5953bb469..46c304003 100644 --- a/crates/interledger-settlement-engines/tests/eth_ledger_settlement.rs +++ b/crates/interledger-settlement-engines/tests/eth_ledger_settlement.rs @@ -223,9 +223,9 @@ fn eth_ledger_settlement() { .and_then(move |_| send1) .and_then(move |_| { get_balance(1, node1_http, "bob").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"10\"}"); + assert_eq!(ret, 10); get_balance(1, node2_http, "alice").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"-10\"}"); + assert_eq!(ret, -10); Ok(()) }) }) @@ -233,9 +233,9 @@ fn eth_ledger_settlement() { .and_then(move |_| send2) .and_then(move |_| { get_balance(1, node1_http, "bob").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"30\"}"); + assert_eq!(ret, 30); get_balance(1, node2_http, "alice").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"-30\"}"); + assert_eq!(ret, -30); Ok(()) }) }) @@ -243,9 +243,9 @@ fn eth_ledger_settlement() { .and_then(move |_| send3) .and_then(move |_| { get_balance(1, node1_http, "bob").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"70\"}"); + assert_eq!(ret, 70); get_balance(1, node2_http, "alice").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"-70\"}"); + assert_eq!(ret, -70); Ok(()) }) }) @@ -261,9 +261,9 @@ fn eth_ledger_settlement() { // Since the credit connection reached -71, and the // settle_to is -10, a 61 Wei transaction is made. get_balance(1, node1_http, "bob").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"10\"}"); + assert_eq!(ret, 10); get_balance(1, node2_http, "alice").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"-10\"}"); + assert_eq!(ret, -10); ganache_pid.kill().unwrap(); Ok(()) }) diff --git a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs index 11dc970be..2f1511d91 100644 --- a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs +++ b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs @@ -343,12 +343,12 @@ fn eth_xrp_interoperable() { get_balance(1, node3_http, "admin"), ]) .and_then(move |balances| { - assert_eq!(balances[0], "{\"balance\":\"-71\"}"); // alice has in total paid 71 - assert_eq!(balances[1], "{\"balance\":\"-10\"}"); // alice owes bob 10 - assert_eq!(balances[2], "{\"balance\":\"-71\"}"); // alice has in total paid 71 - assert_eq!(balances[3], "{\"balance\":\"-10\"}"); // bob owes 10 to bob.charlie - assert_eq!(balances[4], "{\"balance\":\"71\"}"); // bob.charlie has been paid 71 - assert_eq!(balances[5], "{\"balance\":\"10\"}"); // bob.charlie is owed 10 by bob + assert_eq!(balances[0], -71); // alice has in total paid 71 + assert_eq!(balances[1], -10); // alice owes bob 10 + assert_eq!(balances[2], -71); // alice has in total paid 71 + assert_eq!(balances[3], -10); // bob owes 10 to bob.charlie + assert_eq!(balances[4], 71); // bob.charlie has been paid 71 + assert_eq!(balances[5], 10); // bob.charlie is owed 10 by bob node2_engine_redis.kill().unwrap(); node3_engine_redis.kill().unwrap(); diff --git a/crates/interledger-settlement-engines/tests/test_helpers.rs b/crates/interledger-settlement-engines/tests/test_helpers.rs index eba9dd897..01a47bfea 100644 --- a/crates/interledger-settlement-engines/tests/test_helpers.rs +++ b/crates/interledger-settlement-engines/tests/test_helpers.rs @@ -19,6 +19,11 @@ pub struct DeliveryData { pub delivered_amount: u64, } +#[derive(serde::Deserialize)] +pub struct BalanceData { + pub balance: String, +} + #[allow(unused)] pub fn start_ganache() -> std::process::Child { let mut ganache = Command::new("ganache-cli"); @@ -122,7 +127,8 @@ pub fn send_money( }) .and_then(move |body| { let ret: DeliveryData = serde_json::from_slice(&body).unwrap(); - assert_eq!(ret.delivered_amount, amount); + // this does not work if they have different scales + // assert_eq!(ret.delivered_amount, amount); Ok(()) }) } @@ -131,7 +137,7 @@ pub fn get_balance( account_id: u64, node_port: u16, admin_token: &str, -) -> impl Future { +) -> impl Future { let client = reqwest::r#async::Client::new(); client .get(&format!( @@ -145,5 +151,8 @@ pub fn get_balance( .map_err(|err| { eprintln!("Error getting account data: {:?}", err); }) - .and_then(|chunk| Ok(str::from_utf8(&chunk).unwrap().to_string())) + .and_then(|body| { + let ret: BalanceData = serde_json::from_slice(&body).unwrap(); + Ok(ret.balance.parse().unwrap()) + }) } diff --git a/crates/interledger-settlement-engines/tests/xrp_ledger_settlement.rs b/crates/interledger-settlement-engines/tests/xrp_ledger_settlement.rs index e0432e00a..7d31c1f39 100644 --- a/crates/interledger-settlement-engines/tests/xrp_ledger_settlement.rs +++ b/crates/interledger-settlement-engines/tests/xrp_ledger_settlement.rs @@ -222,9 +222,9 @@ fn xrp_ledger_settlement() { .and_then(move |_| send1) .and_then(move |_| { get_balance(1, node1_http, "bob").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"10\"}"); + assert_eq!(ret, 10); get_balance(1, node2_http, "alice").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"-10\"}"); + assert_eq!(ret, -10); Ok(()) }) }) @@ -232,9 +232,9 @@ fn xrp_ledger_settlement() { .and_then(move |_| send2) .and_then(move |_| { get_balance(1, node1_http, "bob").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"30\"}"); + assert_eq!(ret, 30); get_balance(1, node2_http, "alice").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"-30\"}"); + assert_eq!(ret, -30); Ok(()) }) }) @@ -242,9 +242,9 @@ fn xrp_ledger_settlement() { .and_then(move |_| send3) .and_then(move |_| { get_balance(1, node1_http, "bob").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"70\"}"); + assert_eq!(ret, 70); get_balance(1, node2_http, "alice").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"-70\"}"); + assert_eq!(ret, -70); Ok(()) }) }) @@ -257,7 +257,7 @@ fn xrp_ledger_settlement() { // Since the credit connection reached -71, and the // settle_to is -10, a 61 drops transaction is made. get_balance(1, node1_http, "bob").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"10\"}"); + assert_eq!(ret, 10); // Wait a few seconds so that the receiver's engine // gets the data and applies it (longer than the // Ethereum engine since we're using a public @@ -266,7 +266,7 @@ fn xrp_ledger_settlement() { .map_err(move |_| panic!("Weird error.")) .and_then(move |_| { get_balance(1, node2_http, "alice").and_then(move |ret| { - assert_eq!(ret, "{\"balance\":\"-10\"}"); + assert_eq!(ret, -10); alice_engine_redis.kill().unwrap(); engine_alice.kill().unwrap(); bob_engine_redis.kill().unwrap(); From 9395f53c0ef5a65fb7e74cc06aff23e5fbf0008a Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Mon, 5 Aug 2019 13:26:48 +0300 Subject: [PATCH 06/22] test(eth-se): adjust test for proper scale conversion --- .../src/engines/ethereum_ledger/eth_engine.rs | 4 +- .../tests/eth_xrp_interoperable.rs | 42 +++++++++---------- .../tests/test_helpers.rs | 11 +++-- 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs index dcfb0df5a..9cda2de77 100644 --- a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs +++ b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs @@ -945,7 +945,7 @@ mod tests { let bob_mock = mockito::mock("POST", "/accounts/42/settlements") .match_body(mockito::Matcher::JsonString( - "{\"amount\": \"100\", \"scale\": 18 }".to_string(), + "{\"amount\": \"100000000000\", \"scale\": 18 }".to_string(), )) .with_status(200) .with_body("OK".to_string()) @@ -969,7 +969,7 @@ mod tests { ); let ret = - block_on(alice_engine.send_money(bob.id.to_string(), Quantity::new(100, 6))).unwrap(); + block_on(alice_engine.send_money(bob.id.to_string(), Quantity::new(100, 9))).unwrap(); assert_eq!(ret.0.as_u16(), 200); assert_eq!(ret.1, "OK"); diff --git a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs index 2f1511d91..171f827ba 100644 --- a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs +++ b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs @@ -17,11 +17,9 @@ use redis_helpers::*; mod test_helpers; use test_helpers::{ create_account, get_balance, send_money, start_eth_engine, start_ganache, start_xrp_engine, - XRP_DECIMALS, + ETH_DECIMALS, XRP_DECIMALS, }; -const ETH_DECIMALS: u8 = 6; - #[test] fn eth_xrp_interoperable() { // Nodes 1 and 2 are peers, Node 2 is the parent of Node 3 @@ -146,9 +144,9 @@ fn eth_xrp_interoperable() { http_incoming_token: Some("bob".to_string()), http_outgoing_token: Some("alice".to_string()), max_packet_amount: u64::max_value(), - min_balance: Some(-100), - settle_threshold: Some(70), - settle_to: Some(-10), + min_balance: Some(-100000), + settle_threshold: Some(70000), + settle_to: Some(-1000), send_routes: true, receive_routes: true, routing_relation: Some("Peer".to_string()), @@ -188,9 +186,9 @@ fn eth_xrp_interoperable() { http_incoming_token: Some("alice".to_string()), http_outgoing_token: Some("bob".to_string()), max_packet_amount: u64::max_value(), - min_balance: Some(-100), - settle_threshold: Some(70), - settle_to: Some(-10), + min_balance: Some(-100000), + settle_threshold: None, + settle_to: None, send_routes: true, receive_routes: true, routing_relation: Some("Peer".to_string()), @@ -213,7 +211,7 @@ fn eth_xrp_interoperable() { max_packet_amount: u64::max_value(), min_balance: Some(-100), settle_threshold: Some(70), - settle_to: Some(-10), + settle_to: Some(-5), send_routes: false, receive_routes: true, routing_relation: Some("Child".to_string()), @@ -275,7 +273,7 @@ fn eth_xrp_interoperable() { max_packet_amount: u64::max_value(), min_balance: None, settle_threshold: None, - settle_to: Some(-10), + settle_to: None, send_routes: false, receive_routes: false, routing_relation: None, @@ -297,8 +295,8 @@ fn eth_xrp_interoperable() { http_outgoing_token: Some("charlie".to_string()), max_packet_amount: u64::max_value(), min_balance: Some(-100), - settle_threshold: Some(70), - settle_to: Some(-10), + settle_threshold: None, + settle_to: None, send_routes: true, receive_routes: false, routing_relation: Some("Parent".to_string()), @@ -328,9 +326,11 @@ fn eth_xrp_interoperable() { .and_then(move |_| create_account(node2_engine, "0")) .and_then(move |_| create_account(node2_xrp_engine_port, "1")) .and_then(move |_| create_account(node3_xrp_engine_port, "1")) - .and_then(move |_| send_money(node1_http, node3_http, 70, "in_alice")) - // node 3 updates account 1, - .and_then(move |_| send_money(node1_http, node3_http, 1, "in_alice")) + // Pay 70k Gwei --> 70 drops + .and_then(move |_| send_money(node1_http, node3_http, 70000, "in_alice")) + // Pay 1k Gwei --> 1 drop + // This will trigger a 71 Gwei settlement from Alice to Bob. + .and_then(move |_| send_money(node1_http, node3_http, 1000, "in_alice")) .and_then(move |_| { // wait for the settlements delay(10000).map_err(|err| panic!(err)).and_then(move |_| { @@ -343,12 +343,12 @@ fn eth_xrp_interoperable() { get_balance(1, node3_http, "admin"), ]) .and_then(move |balances| { - assert_eq!(balances[0], -71); // alice has in total paid 71 - assert_eq!(balances[1], -10); // alice owes bob 10 - assert_eq!(balances[2], -71); // alice has in total paid 71 - assert_eq!(balances[3], -10); // bob owes 10 to bob.charlie + assert_eq!(balances[0], -71000); // alice has in total paid 71k + assert_eq!(balances[1], -1000); // alice owes bob 1k + assert_eq!(balances[2], 1000); // bob is owed 1k by alice + assert_eq!(balances[3], -5); // bob owes 5 to bob.charlie assert_eq!(balances[4], 71); // bob.charlie has been paid 71 - assert_eq!(balances[5], 10); // bob.charlie is owed 10 by bob + assert_eq!(balances[5], 5); // bob.charlie is owed 10 by bob node2_engine_redis.kill().unwrap(); node3_engine_redis.kill().unwrap(); diff --git a/crates/interledger-settlement-engines/tests/test_helpers.rs b/crates/interledger-settlement-engines/tests/test_helpers.rs index 01a47bfea..334c1fad6 100644 --- a/crates/interledger-settlement-engines/tests/test_helpers.rs +++ b/crates/interledger-settlement-engines/tests/test_helpers.rs @@ -10,7 +10,8 @@ use std::time::Duration; const CONFIRMATIONS: u8 = 0; const CHAIN_ID: u8 = 1; -pub const ETH_DECIMALS: u8 = 18; +#[allow(unused)] +pub const ETH_DECIMALS: u8 = 9; #[allow(unused)] pub const XRP_DECIMALS: u8 = 6; @@ -76,7 +77,7 @@ pub fn start_eth_engine( key, CHAIN_ID, CONFIRMATIONS, - ETH_DECIMALS, + 18, 1000, format!("http://127.0.0.1:{}", settlement_port), None, @@ -110,7 +111,7 @@ pub fn send_money( to: u16, amount: u64, auth: &str, -) -> impl Future { +) -> impl Future { let client = reqwest::r#async::Client::new(); client .post(&format!("http://localhost:{}/pay", from)) @@ -127,9 +128,7 @@ pub fn send_money( }) .and_then(move |body| { let ret: DeliveryData = serde_json::from_slice(&body).unwrap(); - // this does not work if they have different scales - // assert_eq!(ret.delivered_amount, amount); - Ok(()) + Ok(ret.delivered_amount) }) } From 0041bd77b9e855cfd820fe8db972441110c7c035 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Mon, 5 Aug 2019 15:45:31 +0300 Subject: [PATCH 07/22] test(eth-xrp) Set ETHXRP exchange rate --- .../tests/eth_xrp_interoperable.rs | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs index 171f827ba..333f4b804 100644 --- a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs +++ b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs @@ -210,8 +210,8 @@ fn eth_xrp_interoperable() { http_outgoing_token: Some("bob".to_string()), max_packet_amount: u64::max_value(), min_balance: Some(-100), - settle_threshold: Some(70), - settle_to: Some(-5), + settle_threshold: Some(70000), + settle_to: Some(-5000), send_routes: false, receive_routes: true, routing_relation: Some("Child".to_string()), @@ -232,7 +232,8 @@ fn eth_xrp_interoperable() { client .put(&format!("http://localhost:{}/rates", node2_http)) .header("Authorization", "Bearer admin") - .json(&json!({"XRP": 1, "ETH": 1})) + // Let's say 1 ETH = 0.001 XRP for this example + .json(&json!({"XRP": 1000, "ETH": 1})) .send() .map_err(|err| panic!(err)) .and_then(|res| { @@ -294,7 +295,7 @@ fn eth_xrp_interoperable() { http_incoming_token: Some("bob".to_string()), http_outgoing_token: Some("charlie".to_string()), max_packet_amount: u64::max_value(), - min_balance: Some(-100), + min_balance: Some(-100000), settle_threshold: None, settle_to: None, send_routes: true, @@ -343,12 +344,12 @@ fn eth_xrp_interoperable() { get_balance(1, node3_http, "admin"), ]) .and_then(move |balances| { - assert_eq!(balances[0], -71000); // alice has in total paid 71k - assert_eq!(balances[1], -1000); // alice owes bob 1k - assert_eq!(balances[2], 1000); // bob is owed 1k by alice - assert_eq!(balances[3], -5); // bob owes 5 to bob.charlie - assert_eq!(balances[4], 71); // bob.charlie has been paid 71 - assert_eq!(balances[5], 5); // bob.charlie is owed 10 by bob + assert_eq!(balances[0], -71000); // alice has in total paid 71k Gwei + assert_eq!(balances[1], -1000); // alice owes bob 1k Gwei + assert_eq!(balances[2], 1000); // bob is owed 1k Gwei by alice + assert_eq!(balances[3], -5000); // bob owes 5k drops to bob.charlie + assert_eq!(balances[4], 71000); // bob.charlie has been paid 71k drops + assert_eq!(balances[5], 5000); // bob.charlie is owed 5k drops by bob node2_engine_redis.kill().unwrap(); node3_engine_redis.kill().unwrap(); From c32f27c607138f4ba2d0a760a0440cfadb57a5c5 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Mon, 5 Aug 2019 15:54:30 +0300 Subject: [PATCH 08/22] test(eth-xrp): Switch back to BTP --- .../tests/eth_xrp_interoperable.rs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs index 333f4b804..65b081768 100644 --- a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs +++ b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs @@ -144,7 +144,7 @@ fn eth_xrp_interoperable() { http_incoming_token: Some("bob".to_string()), http_outgoing_token: Some("alice".to_string()), max_packet_amount: u64::max_value(), - min_balance: Some(-100000), + min_balance: Some(-100_000), settle_threshold: Some(70000), settle_to: Some(-1000), send_routes: true, @@ -186,7 +186,7 @@ fn eth_xrp_interoperable() { http_incoming_token: Some("alice".to_string()), http_outgoing_token: Some("bob".to_string()), max_packet_amount: u64::max_value(), - min_balance: Some(-100000), + min_balance: Some(-100_000), settle_threshold: None, settle_to: None, send_routes: true, @@ -203,11 +203,11 @@ fn eth_xrp_interoperable() { ilp_address: Address::from_str("example.bob.charlie").unwrap(), asset_code: "XRP".to_string(), asset_scale: XRP_DECIMALS, - btp_incoming_token: None, + btp_incoming_token: Some("charlie".to_string()), btp_uri: None, - http_endpoint: Some(format!("http://localhost:{}/ilp", node3_http)), - http_incoming_token: Some("charlie".to_string()), - http_outgoing_token: Some("bob".to_string()), + http_endpoint: None, + http_incoming_token: None, + http_outgoing_token: None, max_packet_amount: u64::max_value(), min_balance: Some(-100), settle_threshold: Some(70000), @@ -268,9 +268,9 @@ fn eth_xrp_interoperable() { asset_scale: XRP_DECIMALS, btp_incoming_token: None, btp_uri: None, - http_endpoint: Some(format!("http://localhost:{}/ilp", node3_http)), + http_endpoint: None, http_incoming_token: Some("in_charlie".to_string()), - http_outgoing_token: Some("out_charlie".to_string()), + http_outgoing_token: None, max_packet_amount: u64::max_value(), min_balance: None, settle_threshold: None, @@ -290,12 +290,12 @@ fn eth_xrp_interoperable() { asset_code: "XRP".to_string(), asset_scale: XRP_DECIMALS, btp_incoming_token: None, - btp_uri: None, - http_endpoint: Some(format!("http://localhost:{}/ilp", node2_http)), - http_incoming_token: Some("bob".to_string()), - http_outgoing_token: Some("charlie".to_string()), + btp_uri: Some(format!("btp+ws://:charlie@localhost:{}", node2_btp)), + http_endpoint: None, + http_incoming_token: None, + http_outgoing_token: None, max_packet_amount: u64::max_value(), - min_balance: Some(-100000), + min_balance: Some(-100_000), settle_threshold: None, settle_to: None, send_routes: true, From 5b1e2a576a761853b0db90e8cfef00e1de3069bf Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Mon, 5 Aug 2019 16:11:49 +0300 Subject: [PATCH 09/22] fix(crate): Remove settlement_engine_asset_scale from account --- crates/interledger-api/src/lib.rs | 1 - .../tests/eth_ledger_settlement.rs | 4 -- .../tests/eth_xrp_interoperable.rs | 6 --- .../tests/test_helpers.rs | 5 +-- .../tests/xrp_ledger_settlement.rs | 4 -- crates/interledger-settlement/src/api.rs | 2 +- crates/interledger-settlement/src/lib.rs | 6 --- .../src/test_helpers.rs | 1 - crates/interledger-store-redis/src/account.rs | 21 +--------- .../tests/common/fixtures.rs | 3 -- .../tests/routing_test.rs | 1 - crates/interledger/src/main.rs | 1 - crates/interledger/tests/btp_end_to_end.rs | 2 - crates/interledger/tests/three_nodes.rs | 6 --- examples/eth-settlement/README.md | 4 +- examples/eth-settlement/settlements_test.sh | 4 +- examples/eth_xrp_three_nodes/README.md | 39 +++++++++++-------- examples/eth_xrp_three_nodes/interoperable.sh | 8 ++-- examples/xrp_ledger/README.md | 4 +- examples/xrp_ledger/xrp_settlements_test.sh | 4 +- 20 files changed, 39 insertions(+), 87 deletions(-) diff --git a/crates/interledger-api/src/lib.rs b/crates/interledger-api/src/lib.rs index b818f8ae4..859b2857c 100644 --- a/crates/interledger-api/src/lib.rs +++ b/crates/interledger-api/src/lib.rs @@ -71,7 +71,6 @@ pub struct AccountDetails { pub amount_per_minute_limit: Option, pub packets_per_minute_limit: Option, pub settlement_engine_url: Option, - pub settlement_engine_asset_scale: Option, } pub struct NodeApi { diff --git a/crates/interledger-settlement-engines/tests/eth_ledger_settlement.rs b/crates/interledger-settlement-engines/tests/eth_ledger_settlement.rs index 46c304003..59d255b0c 100644 --- a/crates/interledger-settlement-engines/tests/eth_ledger_settlement.rs +++ b/crates/interledger-settlement-engines/tests/eth_ledger_settlement.rs @@ -93,7 +93,6 @@ fn eth_ledger_settlement() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }) .and_then(move |_| { node1_clone.insert_account(AccountDetails { @@ -119,7 +118,6 @@ fn eth_ledger_settlement() { "http://localhost:{}", node1_engine )), - settlement_engine_asset_scale: Some(ETH_DECIMALS), }) }) .and_then(move |_| node1.serve()) @@ -163,7 +161,6 @@ fn eth_ledger_settlement() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }) .and_then(move |_| { node2 @@ -190,7 +187,6 @@ fn eth_ledger_settlement() { "http://localhost:{}", node2_engine )), - settlement_engine_asset_scale: Some(ETH_DECIMALS), }) .and_then(move |_| node2.serve()) }) diff --git a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs index 65b081768..e336ac4e2 100644 --- a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs +++ b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs @@ -129,7 +129,6 @@ fn eth_xrp_interoperable() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }) .and_then(move |_| // TODO insert the accounts via HTTP request @@ -154,7 +153,6 @@ fn eth_xrp_interoperable() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: Some(format!("http://localhost:{}", node1_engine)), - settlement_engine_asset_scale: Some(18), })) .and_then(move |_| node1.serve()) }), @@ -196,7 +194,6 @@ fn eth_xrp_interoperable() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: Some(format!("http://localhost:{}", node2_engine)), - settlement_engine_asset_scale: Some(ETH_DECIMALS), }) .and_then(move |_| { node2_clone.insert_account(AccountDetails { @@ -222,7 +219,6 @@ fn eth_xrp_interoperable() { "http://localhost:{}", node2_xrp_engine_port )), - settlement_engine_asset_scale: Some(XRP_DECIMALS), }) }) }) @@ -282,7 +278,6 @@ fn eth_xrp_interoperable() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }) .and_then(move |_| { node3_clone.insert_account(AccountDetails { @@ -308,7 +303,6 @@ fn eth_xrp_interoperable() { "http://localhost:{}", node3_xrp_engine_port )), - settlement_engine_asset_scale: Some(XRP_DECIMALS), }) }) }) diff --git a/crates/interledger-settlement-engines/tests/test_helpers.rs b/crates/interledger-settlement-engines/tests/test_helpers.rs index 334c1fad6..3e898ec3f 100644 --- a/crates/interledger-settlement-engines/tests/test_helpers.rs +++ b/crates/interledger-settlement-engines/tests/test_helpers.rs @@ -100,10 +100,7 @@ pub fn create_account( .map_err(|err| { eprintln!("Error creating account: {:?}", err); }) - .and_then(move |chunk| { - println!("GOT RES {} {} {:?}", engine_port, account_id, chunk); - Ok(str::from_utf8(&chunk).unwrap().to_string()) - }) + .and_then(move |chunk| Ok(str::from_utf8(&chunk).unwrap().to_string())) } pub fn send_money( diff --git a/crates/interledger-settlement-engines/tests/xrp_ledger_settlement.rs b/crates/interledger-settlement-engines/tests/xrp_ledger_settlement.rs index 7d31c1f39..1d78ee811 100644 --- a/crates/interledger-settlement-engines/tests/xrp_ledger_settlement.rs +++ b/crates/interledger-settlement-engines/tests/xrp_ledger_settlement.rs @@ -104,7 +104,6 @@ fn xrp_ledger_settlement() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }) .and_then(move |_| { node1_clone.insert_account(AccountDetails { @@ -127,7 +126,6 @@ fn xrp_ledger_settlement() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: Some(format!("http://localhost:{}", node1_engine)), - settlement_engine_asset_scale: Some(XRP_DECIMALS), }) }) .and_then(move |_| node1.serve()), @@ -167,7 +165,6 @@ fn xrp_ledger_settlement() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }) .and_then(move |_| { node2 @@ -191,7 +188,6 @@ fn xrp_ledger_settlement() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: Some(format!("http://localhost:{}", node2_engine)), - settlement_engine_asset_scale: Some(XRP_DECIMALS), }) .and_then(move |_| node2.serve()) }), diff --git a/crates/interledger-settlement/src/api.rs b/crates/interledger-settlement/src/api.rs index 394268b09..78a795c0b 100644 --- a/crates/interledger-settlement/src/api.rs +++ b/crates/interledger-settlement/src/api.rs @@ -2,12 +2,12 @@ use super::{ Convert, ConvertDetails, IdempotentData, IdempotentStore, Quantity, SettlementAccount, SettlementStore, SE_ILP_ADDRESS, }; +use bigint::uint::U256 as BigU256; use bytes::Bytes; use futures::{ future::{err, ok, result, Either}, Future, }; -use bigint::uint::U256 as BigU256; use hyper::{Response, StatusCode}; use interledger_ildcp::IldcpAccount; use interledger_packet::PrepareBuilder; diff --git a/crates/interledger-settlement/src/lib.rs b/crates/interledger-settlement/src/lib.rs index 1f71202c4..f4b73b0ad 100644 --- a/crates/interledger-settlement/src/lib.rs +++ b/crates/interledger-settlement/src/lib.rs @@ -48,12 +48,6 @@ impl Quantity { pub struct SettlementEngineDetails { /// Base URL of the settlement engine pub url: Url, - /// Asset scale that the settlement engine is configured to use. - /// For example, sending a settlement for amount 1000 to a settlement engine - /// that uses as scale of 3 would mean that it should send 1 whole unit of that asset. - /// The SettlementClient translates the amounts used for each account internally within - /// Interledger.rs into the correct scale used by the settlement engine. - pub asset_scale: u8, } pub trait SettlementAccount: Account { diff --git a/crates/interledger-settlement/src/test_helpers.rs b/crates/interledger-settlement/src/test_helpers.rs index 0aa985cc8..25c4a50d3 100644 --- a/crates/interledger-settlement/src/test_helpers.rs +++ b/crates/interledger-settlement/src/test_helpers.rs @@ -43,7 +43,6 @@ impl SettlementAccount for TestAccount { } Some(SettlementEngineDetails { url: self.url.clone(), - asset_scale: 9, }) } } diff --git a/crates/interledger-store-redis/src/account.rs b/crates/interledger-store-redis/src/account.rs index 3dcd15604..32e0d9770 100644 --- a/crates/interledger-store-redis/src/account.rs +++ b/crates/interledger-store-redis/src/account.rs @@ -55,7 +55,6 @@ pub struct Account { pub(crate) amount_per_minute_limit: Option, #[serde(serialize_with = "optional_url_to_string")] pub(crate) settlement_engine_url: Option, - pub(crate) settlement_engine_asset_scale: Option, } fn address_to_string(address: &Address, serializer: S) -> Result @@ -147,7 +146,6 @@ impl Account { packets_per_minute_limit: details.packets_per_minute_limit, amount_per_minute_limit: details.amount_per_minute_limit, settlement_engine_url, - settlement_engine_asset_scale: details.settlement_engine_asset_scale, }) } @@ -258,10 +256,6 @@ impl ToRedisArgs for AccountWithEncryptedTokens { "settlement_engine_url".write_redis_args(&mut rv); settlement_engine_url.as_str().write_redis_args(&mut rv); } - if let Some(settlement_engine_asset_scale) = account.settlement_engine_asset_scale { - "settlement_engine_asset_scale".write_redis_args(&mut rv); - settlement_engine_asset_scale.write_redis_args(&mut rv); - } debug_assert!(rv.len() <= ACCOUNT_DETAILS_FIELDS * 2); debug_assert!((rv.len() % 2) == 0); @@ -307,10 +301,6 @@ impl FromRedisValue for AccountWithEncryptedTokens { packets_per_minute_limit: get_value_option("packets_per_minute_limit", &hash)?, amount_per_minute_limit: get_value_option("amount_per_minute_limit", &hash)?, settlement_engine_url: get_url_option("settlement_engine_url", &hash)?, - settlement_engine_asset_scale: get_value_option( - "settlement_engine_asset_scale", - &hash, - )?, }, }) } @@ -461,14 +451,8 @@ impl RateLimitAccount for Account { impl SettlementAccount for Account { fn settlement_engine_details(&self) -> Option { - match ( - &self.settlement_engine_url, - self.settlement_engine_asset_scale, - ) { - (Some(url), Some(asset_scale)) => Some(SettlementEngineDetails { - url: url.clone(), - asset_scale, - }), + match &self.settlement_engine_url { + Some(url) => Some(SettlementEngineDetails { url: url.clone() }), _ => None, } } @@ -499,7 +483,6 @@ mod redis_account { round_trip_time: Some(600), amount_per_minute_limit: None, packets_per_minute_limit: None, - settlement_engine_asset_scale: None, settlement_engine_url: None, }; } diff --git a/crates/interledger-store-redis/tests/common/fixtures.rs b/crates/interledger-store-redis/tests/common/fixtures.rs index 93454e8f4..c0b50249b 100644 --- a/crates/interledger-store-redis/tests/common/fixtures.rs +++ b/crates/interledger-store-redis/tests/common/fixtures.rs @@ -24,7 +24,6 @@ lazy_static! { amount_per_minute_limit: Some(1000), packets_per_minute_limit: Some(2), settlement_engine_url: None, - settlement_engine_asset_scale: None, }; pub static ref ACCOUNT_DETAILS_1: AccountDetails = AccountDetails { ilp_address: Address::from_str("example.bob").unwrap(), @@ -46,7 +45,6 @@ lazy_static! { amount_per_minute_limit: Some(1000), packets_per_minute_limit: Some(20), settlement_engine_url: None, - settlement_engine_asset_scale: None, }; pub static ref ACCOUNT_DETAILS_2: AccountDetails = AccountDetails { ilp_address: Address::from_str("example.charlie").unwrap(), @@ -68,6 +66,5 @@ lazy_static! { amount_per_minute_limit: None, packets_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }; } diff --git a/crates/interledger-store-redis/tests/routing_test.rs b/crates/interledger-store-redis/tests/routing_test.rs index cf9b303a7..ed08b99d2 100644 --- a/crates/interledger-store-redis/tests/routing_test.rs +++ b/crates/interledger-store-redis/tests/routing_test.rs @@ -53,7 +53,6 @@ fn polls_for_route_updates() { amount_per_minute_limit: None, packets_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }) }) .and_then(move |_| { diff --git a/crates/interledger/src/main.rs b/crates/interledger/src/main.rs index ae6d528e6..1604dcb4f 100644 --- a/crates/interledger/src/main.rs +++ b/crates/interledger/src/main.rs @@ -336,7 +336,6 @@ pub fn main() { amount_per_minute_limit: value_t!(matches, "amount_per_minute_limit", u64) .ok(), settlement_engine_url: None, - settlement_engine_asset_scale: None, }; tokio::run(insert_account_redis(redis_uri, &server_secret, account)); } diff --git a/crates/interledger/tests/btp_end_to_end.rs b/crates/interledger/tests/btp_end_to_end.rs index 7d51a795e..f24f6d317 100644 --- a/crates/interledger/tests/btp_end_to_end.rs +++ b/crates/interledger/tests/btp_end_to_end.rs @@ -56,7 +56,6 @@ fn btp_end_to_end() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }), node.insert_account(AccountDetails { ilp_address: Address::from_str("example.node.two").unwrap(), @@ -78,7 +77,6 @@ fn btp_end_to_end() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }), ]) }); diff --git a/crates/interledger/tests/three_nodes.rs b/crates/interledger/tests/three_nodes.rs index ff19276fb..0d252212a 100644 --- a/crates/interledger/tests/three_nodes.rs +++ b/crates/interledger/tests/three_nodes.rs @@ -77,7 +77,6 @@ fn three_nodes() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }) .and_then(move |_| // TODO insert the accounts via HTTP request @@ -102,7 +101,6 @@ fn three_nodes() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, })) .and_then(move |_| node1.serve()), ); @@ -140,7 +138,6 @@ fn three_nodes() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }), node2.insert_account(AccountDetails { ilp_address: Address::from_str("example.two.three").unwrap(), @@ -162,7 +159,6 @@ fn three_nodes() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }), ]) .and_then(move |_| node2.serve()) @@ -218,7 +214,6 @@ fn three_nodes() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }), node3_clone.insert_account(AccountDetails { ilp_address: Address::from_str("example.two").unwrap(), @@ -240,7 +235,6 @@ fn three_nodes() { packets_per_minute_limit: None, amount_per_minute_limit: None, settlement_engine_url: None, - settlement_engine_asset_scale: None, }), ]) .and_then(move |_| node3.serve()) diff --git a/examples/eth-settlement/README.md b/examples/eth-settlement/README.md index 7b66579e8..aec6a6444 100644 --- a/examples/eth-settlement/README.md +++ b/examples/eth-settlement/README.md @@ -177,7 +177,7 @@ Now we have both Alice and Bob up and running. ### Insert Bob's account to Alice's connector ```bash curl http://localhost:7770/accounts -X POST \ - -d "ilp_address=example.bob&asset_code=ETH&asset_scale=18&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3000&settlement_engine_asset_scale=18&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=alice&settle_threshold=70&min_balance=-100&settle_to=10" \ + -d "ilp_address=example.bob&asset_code=ETH&asset_scale=18&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3000&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=alice&settle_threshold=70&min_balance=-100&settle_to=10" \ -H "Authorization: Bearer hi_alice ``` @@ -199,7 +199,7 @@ __Parameter Explanation:__ ```bash curl http://localhost:8770/accounts -X POST \ - -d "ilp_address=example.alice&asset_code=ETH&asset_scale=18&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3001&settlement_engine_asset_scale=18&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=alice&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=-10" \ + -d "ilp_address=example.alice&asset_code=ETH&asset_scale=18&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3001&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=alice&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=-10" \ -H "Authorization: Bearer hi_bob" ``` diff --git a/examples/eth-settlement/settlements_test.sh b/examples/eth-settlement/settlements_test.sh index bcd1b1b17..cb9bcfb71 100755 --- a/examples/eth-settlement/settlements_test.sh +++ b/examples/eth-settlement/settlements_test.sh @@ -71,7 +71,7 @@ printf "\n---------------------------------------\n" # insert Bob's account details on Alice's connector echo "Initializing Bob's account on Alice's connector (this will not return until Bob adds Alice in his connector)" curl http://localhost:7770/accounts -X POST \ - -d "ilp_address=example.bob&asset_code=ETH&asset_scale=18&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3000&settlement_engine_asset_scale=18&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=alice&settle_threshold=70&min_balance=-100&settle_to=10" \ + -d "ilp_address=example.bob&asset_code=ETH&asset_scale=18&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3000&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=alice&settle_threshold=70&min_balance=-100&settle_to=10" \ -H "Authorization: Bearer hi_alice" & printf "\n---------------------------------------\n" @@ -90,7 +90,7 @@ read -p "Press [Enter] key to continue..." # when setting up an account with another party makes senes to give them some slack if they do not prefund echo "Initializing Alice's account on Bob's connector" curl http://localhost:8770/accounts -X POST \ - -d "ilp_address=example.alice&asset_code=ETH&asset_scale=18&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3001&settlement_engine_asset_scale=18&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=alice&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=-10" \ + -d "ilp_address=example.alice&asset_code=ETH&asset_scale=18&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3001&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=alice&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=-10" \ -H "Authorization: Bearer hi_bob" & sleep 2 diff --git a/examples/eth_xrp_three_nodes/README.md b/examples/eth_xrp_three_nodes/README.md index 9655d8557..3aa14af18 100755 --- a/examples/eth_xrp_three_nodes/README.md +++ b/examples/eth_xrp_three_nodes/README.md @@ -112,11 +112,11 @@ curl http://localhost:7770/accounts -X POST \ 1. Insert Bob's account into Alice's connector. -```bash -curl http://localhost:7770/accounts -X POST \ - -d "ilp_address=example.bob&asset_code=ETH&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3000&settlement_engine_asset_scale=6&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=alice&settle_threshold=70&min_balance=-100&settle_to=10&routing_relation=Peer&receive_routes=true&send_routes=true" \ - -H "Authorization: Bearer hi_alice" & -``` + ```bash + curl http://localhost:7770/accounts -X POST \ + -d "ilp_address=example.bob&asset_code=ETH&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3000&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=alice&settle_threshold=70&min_balance=-100&settle_to=10&routing_relation=Peer&receive_routes=true&send_routes=true" \ + -H "Authorization: Bearer hi_alice" & + ``` All set! Now Alice has her connector, settlement engine and redis store up and running. @@ -174,18 +174,19 @@ RUST_LOG="interledger=debug,interledger=trace" $ILP node --config bob.yml 1. Insert Alice's account on Bob's connector (ETH Peer relation) -```bash -curl http://localhost:8770/accounts -X POST \ - -d "ilp_address=example.alice&asset_code=ETH&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3001&settlement_engine_asset_scale=6&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=alice&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=-10&routing_relation=Peer&receive_routes=true&send_routes=true" \ - -H "Authorization: Bearer hi_bob" -``` + ```bash + curl http://localhost:8770/accounts -X POST \ + -d "ilp_address=example.alice&asset_code=ETH&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3001&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=alice&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=-10&routing_relation=Peer&receive_routes=true&send_routes=true" \ + -H "Authorization: Bearer hi_bob" + ``` 1. Insert Charlie's account details on Bob's connector (XRP Child Relation) -```bash -curl http://localhost:8770/accounts -X POST \ - -d "ilp_address=example.bob.charlie&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3002&settlement_engine_asset_scale=6&http_endpoint=http://127.0.0.1:9770/ilp&http_incoming_token=charlie&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=10&routing_relation=Child&receive_routes=true&send_routes=false" \ - -H "Authorization: Bearer hi_bob" & -``` + + ```bash + curl http://localhost:8770/accounts -X POST \ + -d "ilp_address=example.bob.charlie&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3002&http_endpoint=http://127.0.0.1:9770/ilp&http_incoming_token=charlie&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=10&routing_relation=Child&receive_routes=true&send_routes=false" \ + -H "Authorization: Bearer hi_bob" & + ``` ## 5. Configure Charlie @@ -237,7 +238,13 @@ curl http://localhost:9770/accounts -X POST \ -H "Authorization: Bearer hi_charlie" & ``` -## 6. Set the exchange rate between ETH and XRP on Bob since he's relaying + ```bash + curl http://localhost:9770/accounts -X POST \ + -d "ilp_address=example.bob&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3003&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=charlie&settle_threshold=70&min_balance=-100&settle_to=10&routing_relation=Parent&receive_routes=true&send_routes=false" \ + -H "Authorization: Bearer hi_charlie" & + ``` + +## 6. Set the exchange rate between ETH and XRP on Bob's connector ```bash curl http://localhost:8770/rates -X PUT \ diff --git a/examples/eth_xrp_three_nodes/interoperable.sh b/examples/eth_xrp_three_nodes/interoperable.sh index be29a4ec5..4e9fc9a2d 100755 --- a/examples/eth_xrp_three_nodes/interoperable.sh +++ b/examples/eth_xrp_three_nodes/interoperable.sh @@ -93,7 +93,7 @@ printf "\n---------------------------------------\n" # insert Bob's account details on Alice's connector (ETH Peer relation) echo "Initializing Bob's account on Alice's connector (this will not return until Bob adds Alice in his connector)" curl http://localhost:7770/accounts -X POST \ - -d "ilp_address=example.bob&asset_code=ETH&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3000&settlement_engine_asset_scale=6&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=alice&settle_threshold=70&min_balance=-100&settle_to=10&routing_relation=Peer&receive_routes=true&send_routes=true" \ + -d "ilp_address=example.bob&asset_code=ETH&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3000&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=alice&settle_threshold=70&min_balance=-100&settle_to=10&routing_relation=Peer&receive_routes=true&send_routes=true" \ -H "Authorization: Bearer hi_alice" & printf "\n---------------------------------------\n" @@ -104,13 +104,13 @@ read -p "Press [Enter] key to continue..." # insert Alice's account details on Bob's connector (ETH Peer relation) echo "Initializing Alice's account on Bob's connector (Alice has already added Bob as a peer so they should exchange SE addreses)" curl http://localhost:8770/accounts -X POST \ - -d "ilp_address=example.alice&asset_code=ETH&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3001&settlement_engine_asset_scale=6&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=alice&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=-10&routing_relation=Peer&receive_routes=true&send_routes=true" \ + -d "ilp_address=example.alice&asset_code=ETH&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3001&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=alice&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=-10&routing_relation=Peer&receive_routes=true&send_routes=true" \ -H "Authorization: Bearer hi_bob" # insert Charlie's account details on Bob's connector (XRP Child Relation) echo "Initializing Charlie's account on Bob's connector (this will not return until Charlie adds Bob in his connector)" curl http://localhost:8770/accounts -X POST \ - -d "ilp_address=example.bob.charlie&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3002&settlement_engine_asset_scale=6&http_endpoint=http://127.0.0.1:9770/ilp&http_incoming_token=charlie&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=10&routing_relation=Child&receive_routes=true&send_routes=false" \ + -d "ilp_address=example.bob.charlie&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3002&http_endpoint=http://127.0.0.1:9770/ilp&http_incoming_token=charlie&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=10&routing_relation=Child&receive_routes=true&send_routes=false" \ -H "Authorization: Bearer hi_bob" & sleep 2 @@ -128,7 +128,7 @@ printf "\n---------------------------------------\n" # insert Bob's account details on Alice's connector (XRP Peer relation) echo "Initializing Bob's account on Charlie's connector (this will not return until Bob adds Alice in his connector)" curl http://localhost:9770/accounts -X POST \ - -d "ilp_address=example.bob&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3003&settlement_engine_asset_scale=6&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=charlie&settle_threshold=70&min_balance=-100&settle_to=10&routing_relation=Parent&receive_routes=true&send_routes=false" \ + -d "ilp_address=example.bob&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3003&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=charlie&settle_threshold=70&min_balance=-100&settle_to=10&routing_relation=Parent&receive_routes=true&send_routes=false" \ -H "Authorization: Bearer hi_charlie" & # Set the exchange rate between ETH and XRP diff --git a/examples/xrp_ledger/README.md b/examples/xrp_ledger/README.md index a1bf3f930..f5e821e46 100644 --- a/examples/xrp_ledger/README.md +++ b/examples/xrp_ledger/README.md @@ -149,7 +149,7 @@ Now we have both Alice and Bob up and running. ### Insert Bob's account to Alice's connector ```bash curl http://localhost:7770/accounts -X POST \ - -d "ilp_address=example.bob&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3000&settlement_engine_asset_scale=6&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=alice&settle_threshold=70&min_balance=-100&settle_to=10" \ + -d "ilp_address=example.bob&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3000&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=alice&settle_threshold=70&min_balance=-100&settle_to=10" \ -H "Authorization: Bearer hi_alice ``` @@ -170,7 +170,7 @@ __Parameter Explanation:__ ```bash curl http://localhost:8770/accounts -X POST \ - -d "ilp_address=example.alice&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3001&settlement_engine_asset_scale=6&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=alice&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=-10" \ + -d "ilp_address=example.alice&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3001&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=alice&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=-10" \ -H "Authorization: Bearer hi_bob" ``` diff --git a/examples/xrp_ledger/xrp_settlements_test.sh b/examples/xrp_ledger/xrp_settlements_test.sh index 3cfa92a68..96a76bdca 100644 --- a/examples/xrp_ledger/xrp_settlements_test.sh +++ b/examples/xrp_ledger/xrp_settlements_test.sh @@ -51,7 +51,7 @@ printf "\n---------------------------------------\n" # insert Bob's account details on Alice's connector echo "Initializing Bob's account on Alice's connector (this will not return until Bob adds Alice in his connector)" curl http://localhost:7770/accounts -X POST \ - -d "ilp_address=example.bob&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3000&settlement_engine_asset_scale=6&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=alice&settle_threshold=70&min_balance=-100&settle_to=10" \ + -d "ilp_address=example.bob&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3000&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=alice&settle_threshold=70&min_balance=-100&settle_to=10" \ -H "Authorization: Bearer hi_alice" & printf "\n---------------------------------------\n" @@ -70,7 +70,7 @@ printf "\n---------------------------------------\n" # when setting up an account with another party makes senes to give them some slack if they do not prefund echo "Initializing Alice's account on Bob's connector" curl http://localhost:8770/accounts -X POST \ - -d "ilp_address=example.alice&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3001&settlement_engine_asset_scale=6&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=alice&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=-10" \ + -d "ilp_address=example.alice&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3001&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=alice&http_outgoing_token=bob&settle_threshold=70&min_balance=-100&settle_to=-10" \ -H "Authorization: Bearer hi_bob" & sleep 2 From a5874a9215159bec60cce8e9e5b35655534b7239 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 6 Aug 2019 10:08:54 +0300 Subject: [PATCH 10/22] improvement(settlement/engines): use BigUInt to handle big numbers For the connector: if the number after conversion would still not fit in a u64, we use the u64::MAX --- .../interledger-settlement-engines/Cargo.toml | 2 +- .../src/engines/ethereum_ledger/eth_engine.rs | 18 ++++++++------ crates/interledger-settlement/Cargo.toml | 3 ++- crates/interledger-settlement/src/api.rs | 24 ++++++++++++------- crates/interledger-settlement/src/lib.rs | 13 +++++----- 5 files changed, 35 insertions(+), 25 deletions(-) diff --git a/crates/interledger-settlement-engines/Cargo.toml b/crates/interledger-settlement-engines/Cargo.toml index 0882ecac5..7ed62b447 100644 --- a/crates/interledger-settlement-engines/Cargo.toml +++ b/crates/interledger-settlement-engines/Cargo.toml @@ -34,7 +34,7 @@ http = "0.1.17" clap = "2.32.0" clarity = { git = "https://github.com/gakonst/clarity" } sha3 = "0.8.2" -bigint = "4.4.1" +num-bigint = "0.2.2" [dev-dependencies] lazy_static = "1.3" diff --git a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs index 9cda2de77..88932c1dd 100644 --- a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs +++ b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs @@ -6,7 +6,6 @@ use sha3::{Digest, Keccak256 as Sha3}; use std::collections::HashMap; use std::iter::FromIterator; -use bigint::uint::U256 as BigU256; use ethereum_tx_sign::web3::{ api::Web3, futures::future::{err, join_all, ok, result, Either, Future}, @@ -17,12 +16,12 @@ use ethereum_tx_sign::web3::{ use hyper::StatusCode; use interledger_store_redis::RedisStoreBuilder; use log::info; +use num_bigint::BigUint; use redis::IntoConnectionInfo; use reqwest::r#async::{Client, Response as HttpResponse}; use ring::{digest, hmac}; use serde::{Deserialize, Serialize}; use serde_json::json; -use std::mem; use std::{ marker::PhantomData, str::FromStr, @@ -761,7 +760,7 @@ where let self_clone = self.clone(); let engine_scale = self.asset_scale; Box::new( - result(BigU256::from_dec_str(&body.amount).map_err(move |err| { + result(BigUint::from_str(&body.amount).map_err(move |err| { let error_msg = format!("Error converting to U256 {:?}", err); error!("{:?}", error_msg); (StatusCode::from_u16(400).unwrap(), error_msg) @@ -775,11 +774,16 @@ where from: engine_scale, to: body.scale, }); - // Typecast to use the Convert trait implemented for - // bigint::uint::U256 while we're using ethereum_types::U256 - // from rust-web3 which just re-exports it. Should be safe. - let amount: U256 = unsafe { mem::transmute(amount) }; + // Typecast from num_bigint::BigUInt because we're using + // ethereum_types::U256 for all rust-web3 related functionality + result(U256::from_dec_str(&amount.to_string()).map_err(move |err| { + let error_msg = format!("Error converting to U256 {:?}", err); + error!("{:?}", error_msg); + (StatusCode::from_u16(400).unwrap(), error_msg) + })) + }) + .and_then(move |amount| { self_clone .load_account(account_id) .map_err(move |err| { diff --git a/crates/interledger-settlement/Cargo.toml b/crates/interledger-settlement/Cargo.toml index f806ea628..b9695a12e 100644 --- a/crates/interledger-settlement/Cargo.toml +++ b/crates/interledger-settlement/Cargo.toml @@ -26,7 +26,8 @@ rand = "0.6.5" ring = "0.14.6" tokio-retry = "0.2.0" tokio = "0.1.20" -bigint = "4.4.1" +num-bigint = "0.2.2" +num-traits = "0.2.8" [dev-dependencies] parking_lot = "0.8.0" diff --git a/crates/interledger-settlement/src/api.rs b/crates/interledger-settlement/src/api.rs index 78a795c0b..ae5d822d0 100644 --- a/crates/interledger-settlement/src/api.rs +++ b/crates/interledger-settlement/src/api.rs @@ -2,7 +2,6 @@ use super::{ Convert, ConvertDetails, IdempotentData, IdempotentStore, Quantity, SettlementAccount, SettlementStore, SE_ILP_ADDRESS, }; -use bigint::uint::U256 as BigU256; use bytes::Bytes; use futures::{ future::{err, ok, result, Either}, @@ -13,6 +12,8 @@ use interledger_ildcp::IldcpAccount; use interledger_packet::PrepareBuilder; use interledger_service::{AccountStore, OutgoingRequest, OutgoingService}; use log::error; +use num_bigint::BigUint; +use num_traits::cast::ToPrimitive; use ring::digest::{digest, SHA256}; use std::{ marker::PhantomData, @@ -169,7 +170,7 @@ impl_web! { } }) .and_then(move |account| { - result(BigU256::from_dec_str(&amount)) + result(BigUint::from_str(&amount)) .map_err(move |_| { let error_msg = format!("Could not convert amount: {:?}", amount); error!("{}", error_msg); @@ -182,19 +183,24 @@ impl_web! { from: account.asset_scale(), to: engine_scale, }); - // after downscaling it, hopefully we can convert to u64 without - // loss of precision - let amount = amount.low_u64(); - store.update_balance_for_incoming_settlement(account_id, amount, idempotency_key) + // If we'd overflow, settle for the maximum u64 value + let safe_amount = if let Some(amount) = amount.to_u64() { + amount + } else { + std::u64::MAX + }; + store.update_balance_for_incoming_settlement(account_id, safe_amount, idempotency_key) .map_err(move |_| { let error_msg = format!("Error updating balance of account: {} for incoming settlement of amount: {}", account_id, amount); error!("{}", error_msg); (StatusCode::from_u16(500).unwrap(), error_msg) }) + .and_then(move |_| { + // TODO: Return a Quantity of safe_amount and account.asset_scale() + let ret = Bytes::from("Success"); + Ok((StatusCode::OK, ret)) + }) }) - }).and_then(move |_| { - let ret = Bytes::from("Success"); - Ok((StatusCode::OK, ret)) })) } diff --git a/crates/interledger-settlement/src/lib.rs b/crates/interledger-settlement/src/lib.rs index f4b73b0ad..b7afded76 100644 --- a/crates/interledger-settlement/src/lib.rs +++ b/crates/interledger-settlement/src/lib.rs @@ -19,13 +19,13 @@ mod fixtures; mod message_service; #[cfg(test)] mod test_helpers; -use bigint::uint::U256; +use num_bigint::BigUint; +use std::ops::{Div, Mul}; pub use api::SettlementApi; pub use client::SettlementClient; pub use message_service::SettlementMessageService; -use std::ops::Div; lazy_static! { pub static ref SE_ILP_ADDRESS: Address = Address::from_str("peer.settle").unwrap(); } @@ -128,15 +128,14 @@ impl Convert for f64 { } } -impl Convert for U256 { - fn normalize_scale(&self, details: ConvertDetails) -> U256 { +impl Convert for BigUint { + fn normalize_scale(&self, details: ConvertDetails) -> Self { let from_scale = details.from; let to_scale = details.to; if from_scale >= to_scale { - self.overflowing_mul(U256::from(10u64.pow(u32::from(from_scale - to_scale)))) - .0 + self.mul(10u64.pow(u32::from(from_scale - to_scale))) } else { - self.div(U256::from(10u64.pow(u32::from(to_scale - from_scale)))) + self.div(10u64.pow(u32::from(to_scale - from_scale))) } } } From 61ecec50f811c307c87d3de73800e3d8f5ae90b2 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 6 Aug 2019 10:22:32 +0300 Subject: [PATCH 11/22] test(settlement-engines): use asset_scale local variables per test instead of constants --- .../tests/eth_ledger_settlement.rs | 13 ++++++------- .../tests/eth_xrp_interoperable.rs | 17 +++++++++-------- .../tests/test_helpers.rs | 11 ++--------- .../tests/xrp_ledger_settlement.rs | 11 ++++++----- 4 files changed, 23 insertions(+), 29 deletions(-) diff --git a/crates/interledger-settlement-engines/tests/eth_ledger_settlement.rs b/crates/interledger-settlement-engines/tests/eth_ledger_settlement.rs index 59d255b0c..041f480eb 100644 --- a/crates/interledger-settlement-engines/tests/eth_ledger_settlement.rs +++ b/crates/interledger-settlement-engines/tests/eth_ledger_settlement.rs @@ -16,9 +16,7 @@ mod redis_helpers; use redis_helpers::*; mod test_helpers; -use test_helpers::{ - create_account, get_balance, send_money, start_eth_engine, start_ganache, ETH_DECIMALS, -}; +use test_helpers::{create_account, get_balance, send_money, start_eth_engine, start_ganache}; #[test] /// In this test we have Alice and Bob who have peered with each other and run @@ -29,6 +27,7 @@ use test_helpers::{ /// transactions, and once the transaction has sufficient confirmations it /// lets Bob's connector know about it, so that it adjusts their credit. fn eth_ledger_settlement() { + let eth_decimals = 9; // Nodes 1 and 2 are peers, Node 2 is the parent of Node 3 let _ = env_logger::try_init(); let context = TestContext::new(); @@ -76,7 +75,7 @@ fn eth_ledger_settlement() { .insert_account(AccountDetails { ilp_address: Address::from_str("example.alice").unwrap(), asset_code: "ETH".to_string(), - asset_scale: ETH_DECIMALS, + asset_scale: eth_decimals, btp_incoming_token: None, btp_uri: None, http_endpoint: None, @@ -98,7 +97,7 @@ fn eth_ledger_settlement() { node1_clone.insert_account(AccountDetails { ilp_address: Address::from_str("example.bob").unwrap(), asset_code: "ETH".to_string(), - asset_scale: ETH_DECIMALS, + asset_scale: eth_decimals, btp_incoming_token: None, btp_uri: None, http_endpoint: Some(format!("http://localhost:{}/ilp", node2_http)), @@ -144,7 +143,7 @@ fn eth_ledger_settlement() { .insert_account(AccountDetails { ilp_address: Address::from_str("example.bob").unwrap(), asset_code: "ETH".to_string(), - asset_scale: ETH_DECIMALS, + asset_scale: eth_decimals, btp_incoming_token: None, btp_uri: None, http_endpoint: None, @@ -167,7 +166,7 @@ fn eth_ledger_settlement() { .insert_account(AccountDetails { ilp_address: Address::from_str("example.alice").unwrap(), asset_code: "ETH".to_string(), - asset_scale: ETH_DECIMALS, + asset_scale: eth_decimals, btp_incoming_token: None, btp_uri: None, http_endpoint: Some(format!("http://localhost:{}/ilp", node1_http)), diff --git a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs index e336ac4e2..e4bea2fc7 100644 --- a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs +++ b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs @@ -17,11 +17,12 @@ use redis_helpers::*; mod test_helpers; use test_helpers::{ create_account, get_balance, send_money, start_eth_engine, start_ganache, start_xrp_engine, - ETH_DECIMALS, XRP_DECIMALS, }; #[test] fn eth_xrp_interoperable() { + let eth_decimals = 9; + let xrp_decimals = 6; // Nodes 1 and 2 are peers, Node 2 is the parent of Node 3 let _ = env_logger::try_init(); let context = TestContext::new(); @@ -112,7 +113,7 @@ fn eth_xrp_interoperable() { .insert_account(AccountDetails { ilp_address: Address::from_str("example.alice").unwrap(), asset_code: "ETH".to_string(), - asset_scale: ETH_DECIMALS, + asset_scale: eth_decimals, btp_incoming_token: None, btp_uri: None, http_endpoint: Some(format!("http://localhost:{}/ilp", node1_http)), @@ -136,7 +137,7 @@ fn eth_xrp_interoperable() { .insert_account(AccountDetails { ilp_address: Address::from_str("example.bob").unwrap(), asset_code: "ETH".to_string(), - asset_scale: ETH_DECIMALS, + asset_scale: eth_decimals, btp_incoming_token: None, btp_uri: None, http_endpoint: Some(format!("http://localhost:{}/ilp", node2_http)), @@ -177,7 +178,7 @@ fn eth_xrp_interoperable() { .insert_account(AccountDetails { ilp_address: Address::from_str("example.alice").unwrap(), asset_code: "ETH".to_string(), - asset_scale: ETH_DECIMALS, + asset_scale: eth_decimals, btp_incoming_token: None, btp_uri: None, http_endpoint: Some(format!("http://localhost:{}/ilp", node1_http)), @@ -199,7 +200,7 @@ fn eth_xrp_interoperable() { node2_clone.insert_account(AccountDetails { ilp_address: Address::from_str("example.bob.charlie").unwrap(), asset_code: "XRP".to_string(), - asset_scale: XRP_DECIMALS, + asset_scale: xrp_decimals, btp_incoming_token: Some("charlie".to_string()), btp_uri: None, http_endpoint: None, @@ -228,7 +229,7 @@ fn eth_xrp_interoperable() { client .put(&format!("http://localhost:{}/rates", node2_http)) .header("Authorization", "Bearer admin") - // Let's say 1 ETH = 0.001 XRP for this example + // Let's say 0.001 ETH = 1 XRP for this example .json(&json!({"XRP": 1000, "ETH": 1})) .send() .map_err(|err| panic!(err)) @@ -261,7 +262,7 @@ fn eth_xrp_interoperable() { .insert_account(AccountDetails { ilp_address: Address::from_str("example.bob.charlie").unwrap(), asset_code: "XRP".to_string(), - asset_scale: XRP_DECIMALS, + asset_scale: xrp_decimals, btp_incoming_token: None, btp_uri: None, http_endpoint: None, @@ -283,7 +284,7 @@ fn eth_xrp_interoperable() { node3_clone.insert_account(AccountDetails { ilp_address: Address::from_str("example.bob").unwrap(), asset_code: "XRP".to_string(), - asset_scale: XRP_DECIMALS, + asset_scale: xrp_decimals, btp_incoming_token: None, btp_uri: Some(format!("btp+ws://:charlie@localhost:{}", node2_btp)), http_endpoint: None, diff --git a/crates/interledger-settlement-engines/tests/test_helpers.rs b/crates/interledger-settlement-engines/tests/test_helpers.rs index 3e898ec3f..fa4c2fddc 100644 --- a/crates/interledger-settlement-engines/tests/test_helpers.rs +++ b/crates/interledger-settlement-engines/tests/test_helpers.rs @@ -8,13 +8,6 @@ use std::str; use std::thread::sleep; use std::time::Duration; -const CONFIRMATIONS: u8 = 0; -const CHAIN_ID: u8 = 1; -#[allow(unused)] -pub const ETH_DECIMALS: u8 = 9; -#[allow(unused)] -pub const XRP_DECIMALS: u8 = 6; - #[derive(serde::Deserialize)] pub struct DeliveryData { pub delivered_amount: u64, @@ -75,8 +68,8 @@ pub fn start_eth_engine( engine_port, &cli::random_secret(), key, - CHAIN_ID, - CONFIRMATIONS, + 1, + 0, 18, 1000, format!("http://127.0.0.1:{}", settlement_port), diff --git a/crates/interledger-settlement-engines/tests/xrp_ledger_settlement.rs b/crates/interledger-settlement-engines/tests/xrp_ledger_settlement.rs index 1d78ee811..358a541bf 100644 --- a/crates/interledger-settlement-engines/tests/xrp_ledger_settlement.rs +++ b/crates/interledger-settlement-engines/tests/xrp_ledger_settlement.rs @@ -14,7 +14,7 @@ mod redis_helpers; use redis_helpers::*; mod test_helpers; -use test_helpers::{create_account, get_balance, send_money, start_xrp_engine, XRP_DECIMALS}; +use test_helpers::{create_account, get_balance, send_money, start_xrp_engine}; #[test] /// In this test we have Alice and Bob who have peered with each other and run @@ -25,6 +25,7 @@ use test_helpers::{create_account, get_balance, send_money, start_xrp_engine, XR /// transactions, and once the transaction has sufficient confirmations it /// lets Bob's connector know about it, so that it adjusts their credit. fn xrp_ledger_settlement() { + let xrp_decimals = 6; // Nodes 1 and 2 are peers, Node 2 is the parent of Node 3 let _ = env_logger::try_init(); let context = TestContext::new(); @@ -87,7 +88,7 @@ fn xrp_ledger_settlement() { .insert_account(AccountDetails { ilp_address: Address::from_str("example.alice").unwrap(), asset_code: "XRP".to_string(), - asset_scale: XRP_DECIMALS, + asset_scale: xrp_decimals, btp_incoming_token: None, btp_uri: None, http_endpoint: None, @@ -109,7 +110,7 @@ fn xrp_ledger_settlement() { node1_clone.insert_account(AccountDetails { ilp_address: Address::from_str("example.bob").unwrap(), asset_code: "XRP".to_string(), - asset_scale: XRP_DECIMALS, + asset_scale: xrp_decimals, btp_incoming_token: None, btp_uri: None, http_endpoint: Some(format!("http://localhost:{}/ilp", node2_http)), @@ -148,7 +149,7 @@ fn xrp_ledger_settlement() { .insert_account(AccountDetails { ilp_address: Address::from_str("example.bob").unwrap(), asset_code: "XRP".to_string(), - asset_scale: XRP_DECIMALS, + asset_scale: xrp_decimals, btp_incoming_token: None, btp_uri: None, http_endpoint: None, @@ -171,7 +172,7 @@ fn xrp_ledger_settlement() { .insert_account(AccountDetails { ilp_address: Address::from_str("example.alice").unwrap(), asset_code: "XRP".to_string(), - asset_scale: XRP_DECIMALS, + asset_scale: xrp_decimals, btp_incoming_token: None, btp_uri: None, http_endpoint: Some(format!("http://localhost:{}/ilp", node1_http)), From 642a29bf67d73b0d62ebc46c9f46bc68c09fc5ad Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 6 Aug 2019 10:34:38 +0300 Subject: [PATCH 12/22] test(engine-eth-xrp): elaborate on how results are produced --- .../tests/eth_xrp_interoperable.rs | 37 +++++++++++++++---- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs index e4bea2fc7..2087d85d8 100644 --- a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs +++ b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs @@ -146,7 +146,7 @@ fn eth_xrp_interoperable() { max_packet_amount: u64::max_value(), min_balance: Some(-100_000), settle_threshold: Some(70000), - settle_to: Some(-1000), + settle_to: Some(10000), send_routes: true, receive_routes: true, routing_relation: Some("Peer".to_string()), @@ -209,7 +209,7 @@ fn eth_xrp_interoperable() { max_packet_amount: u64::max_value(), min_balance: Some(-100), settle_threshold: Some(70000), - settle_to: Some(-5000), + settle_to: Some(5000), send_routes: false, receive_routes: true, routing_relation: Some("Child".to_string()), @@ -339,12 +339,33 @@ fn eth_xrp_interoperable() { get_balance(1, node3_http, "admin"), ]) .and_then(move |balances| { - assert_eq!(balances[0], -71000); // alice has in total paid 71k Gwei - assert_eq!(balances[1], -1000); // alice owes bob 1k Gwei - assert_eq!(balances[2], 1000); // bob is owed 1k Gwei by alice - assert_eq!(balances[3], -5000); // bob owes 5k drops to bob.charlie - assert_eq!(balances[4], 71000); // bob.charlie has been paid 71k drops - assert_eq!(balances[5], 5000); // bob.charlie is owed 5k drops by bob + // Alice has paid Charlie in total 71k Gwei through Bob. + assert_eq!(balances[0], -71000); + // Since Alice has configured Bob's + // `settle_threshold` and `settle_to` to be + // 70k and -10k respectively, once she + // exceeded the 70k threshold, she made a 61k + // Gwei settlement to Bob so that their debt + // settles down to 10k. + // From her perspective, Bob's account has a + // positive 10k balance since she owes him money. + assert_eq!(balances[1], 10000); + // From Bob's perspective, Alice's account + // has a negative sign since he is owed money. + assert_eq!(balances[2], -10000); + // As Bob forwards money to Charlie, he also + // eventually exceeds the `settle_threshold` + // which incidentally is set to 70k. As a + // result, he must make a XRP ledger + // settlement of 66k Drops to get his debt + // back to the `settle_to` value of charlie, + // which is 5k (71k - 5k = 66k). + assert_eq!(balances[3], 5000); + // Charlie's balance indicates that he's + // received 71k drops from Alice + assert_eq!(balances[4], 71000); + // And he sees is owed 5k by Bob. + assert_eq!(balances[5], -5000); node2_engine_redis.kill().unwrap(); node3_engine_redis.kill().unwrap(); From 725b9f0a71172f534854b7f49f6d67514fac6276 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 6 Aug 2019 11:43:07 +0300 Subject: [PATCH 13/22] fix(settlement): Convert asset scale properly Previously the Convert trait implementation had an arithmetic error, which was forcing us to use provide it parameters in the opposite order. --- .../src/exchange_rates_service.rs | 4 +- .../src/engines/ethereum_ledger/eth_engine.rs | 8 +- crates/interledger-settlement/src/api.rs | 9 +- crates/interledger-settlement/src/lib.rs | 107 +++++++++--------- 4 files changed, 61 insertions(+), 67 deletions(-) diff --git a/crates/interledger-service-util/src/exchange_rates_service.rs b/crates/interledger-service-util/src/exchange_rates_service.rs index df33dcd1c..f3774b2b2 100644 --- a/crates/interledger-service-util/src/exchange_rates_service.rs +++ b/crates/interledger-service-util/src/exchange_rates_service.rs @@ -91,8 +91,8 @@ where }; let scaled_rate = rate.normalize_scale(ConvertDetails { - from: request.to.asset_scale(), - to: request.from.asset_scale(), + from: request.from.asset_scale(), + to: request.to.asset_scale(), }); let outgoing_amount = (request.prepare.amount() as f64) * scaled_rate; request.prepare.set_amount(outgoing_amount as u64); diff --git a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs index 88932c1dd..316773052 100644 --- a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs +++ b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs @@ -765,14 +765,14 @@ where error!("{:?}", error_msg); (StatusCode::from_u16(400).unwrap(), error_msg) })) - .and_then(move |amount| { + .and_then(move |amount_from_connector| { // If we receive a Quantity { amount: "1", scale: 9}, // we must normalize it to our engine's scale (typically 18 // for ETH/ERC20). // Slightly counter-intuitive that `body.scale` is set as `to`. - let amount = amount.normalize_scale(ConvertDetails { - from: engine_scale, - to: body.scale, + let amount = amount_from_connector.normalize_scale(ConvertDetails { + from: body.scale, + to: engine_scale, }); // Typecast from num_bigint::BigUInt because we're using diff --git a/crates/interledger-settlement/src/api.rs b/crates/interledger-settlement/src/api.rs index ae5d822d0..d030f4b2f 100644 --- a/crates/interledger-settlement/src/api.rs +++ b/crates/interledger-settlement/src/api.rs @@ -176,12 +176,11 @@ impl_web! { error!("{}", error_msg); (StatusCode::from_u16(500).unwrap(), error_msg) }) - .and_then(move |amount| { + .and_then(move |amount_from_engine| { let account_id = account.id(); - let amount = amount.normalize_scale(ConvertDetails { - // scale it down so that it fits in the connector - from: account.asset_scale(), - to: engine_scale, + let amount = amount_from_engine.normalize_scale(ConvertDetails { + from: engine_scale, + to: account.asset_scale(), }); // If we'd overflow, settle for the maximum u64 value let safe_amount = if let Some(amount) = amount.to_u64() { diff --git a/crates/interledger-settlement/src/lib.rs b/crates/interledger-settlement/src/lib.rs index b7afded76..8d567dc8e 100644 --- a/crates/interledger-settlement/src/lib.rs +++ b/crates/interledger-settlement/src/lib.rs @@ -106,36 +106,36 @@ pub trait Convert { impl Convert for u64 { fn normalize_scale(&self, details: ConvertDetails) -> Self { - let from_scale = details.from; - let to_scale = details.to; - if from_scale >= to_scale { - self * 10u64.pow(u32::from(from_scale - to_scale)) + let scale_diff = (details.from as i8 - details.to as i8).abs() as u8; + let scale = 10u64.pow(scale_diff.into()); + if details.to >= details.from { + self * scale } else { - self / 10u64.pow(u32::from(to_scale - from_scale)) + self / scale } } } impl Convert for f64 { fn normalize_scale(&self, details: ConvertDetails) -> Self { - let from_scale = details.from; - let to_scale = details.to; - if from_scale >= to_scale { - self * 10f64.powi(i32::from(from_scale - to_scale)) + let scale_diff = (details.from as i8 - details.to as i8).abs() as u8; + let scale = 10f64.powi(scale_diff.into()); + if details.to >= details.from { + self * scale } else { - self / 10f64.powi(i32::from(to_scale - from_scale)) + self / scale } } } impl Convert for BigUint { fn normalize_scale(&self, details: ConvertDetails) -> Self { - let from_scale = details.from; - let to_scale = details.to; - if from_scale >= to_scale { - self.mul(10u64.pow(u32::from(from_scale - to_scale))) + let scale_diff = (details.from as i8 - details.to as i8).abs() as u8; + let scale = 10u64.pow(scale_diff.into()); + if details.to >= details.from { + self.mul(scale) } else { - self.div(10u64.pow(u32::from(to_scale - from_scale))) + self.div(scale) } } } @@ -147,45 +147,40 @@ mod tests { #[test] fn u64_test() { - // 1 unit with base 1, is 1 unit with base 1 + // 1 unit with scale 1, is 1 unit with scale 1 assert_eq!(1u64.normalize_scale(ConvertDetails { from: 1, to: 1 }), 1); - // 1 unit with base 10, is 10 units with base 1 - assert_eq!(1u64.normalize_scale(ConvertDetails { from: 2, to: 1 }), 10); - // 1 wei is 1e9 sats (multiplied by rate) + // there's leftovers for all number slots which do not increase in + // increments of 10^abs(to_scale-from_scale) + assert_eq!(1u64.normalize_scale(ConvertDetails { from: 2, to: 1 }), 0); + // 1 unit with scale 1, is 10 units with scale 2 + assert_eq!(1u64.normalize_scale(ConvertDetails { from: 1, to: 2 }), 10); + // 1 gwei (scale 9) is 1e9 wei (scale 18) assert_eq!( - 1u64.normalize_scale(ConvertDetails { from: 18, to: 9 }), + 1u64.normalize_scale(ConvertDetails { from: 9, to: 18 }), 1_000_000_000 ); - - // there's leftovers for all number slots which do not increase in - // increments of 10^{to_scale-from_scale} - assert_eq!(1u64.normalize_scale(ConvertDetails { from: 1, to: 2 }), 0); - assert_eq!(10u64.normalize_scale(ConvertDetails { from: 1, to: 2 }), 1); - // 100 units with base 2 is 10 units with base 3 + // 1_000_000_000 wei is 1gwei assert_eq!( - 100u64.normalize_scale(ConvertDetails { from: 2, to: 3 }), - 10 + 1_000_000_000u64.normalize_scale(ConvertDetails { from: 18, to: 9 }), + 1, ); - // 299 units with base 2 is 10 units with base 3 plus 99 leftovers + // 10 units with base 2 is 100 units with base 3 assert_eq!( - 100u64.normalize_scale(ConvertDetails { from: 2, to: 3 }), - 10 + 10u64.normalize_scale(ConvertDetails { from: 2, to: 3 }), + 100 ); - - assert_eq!(999u64.normalize_scale(ConvertDetails { from: 6, to: 9 }), 0); + // 299 units with base 3 is 29 units with base 2 (0.9 leftovers) assert_eq!( - 1000u64.normalize_scale(ConvertDetails { from: 6, to: 9 }), - 1 + 299u64.normalize_scale(ConvertDetails { from: 3, to: 2 }), + 29 ); + assert_eq!(999u64.normalize_scale(ConvertDetails { from: 9, to: 6 }), 0); assert_eq!( - 1999u64.normalize_scale(ConvertDetails { from: 6, to: 9 }), + 1000u64.normalize_scale(ConvertDetails { from: 9, to: 6 }), 1 - ); // 5 is leftovers, maybe we should return it? - - // allow making sub-sat micropayments - assert_eq!(1u64.normalize_scale(ConvertDetails { from: 9, to: 18 }), 0); + ); assert_eq!( - 1_000_000_000u64.normalize_scale(ConvertDetails { from: 9, to: 18 }), + 1999u64.normalize_scale(ConvertDetails { from: 9, to: 6 }), 1 ); } @@ -197,42 +192,42 @@ mod tests { assert_eq!(1f64.normalize_scale(ConvertDetails { from: 1, to: 1 }), 1.0); // 1 unit with base 10, is 10 units with base 1 assert_eq!( - 1f64.normalize_scale(ConvertDetails { from: 2, to: 1 }), + 1f64.normalize_scale(ConvertDetails { from: 1, to: 2 }), 10.0 ); - // 1 wei is 1e9 sats (multiplied by rate) + // 1 sat is 1e9 wei (multiplied by rate) assert_eq!( - 1f64.normalize_scale(ConvertDetails { from: 18, to: 9 }), + 1f64.normalize_scale(ConvertDetails { from: 9, to: 18 }), 1_000_000_000.0 ); - // 1.0 unit with base 1 is 0.1 unit with base 2 - assert_eq!(1f64.normalize_scale(ConvertDetails { from: 1, to: 2 }), 0.1); + // 1.0 unit with base 2 is 0.1 unit with base 1 + assert_eq!(1f64.normalize_scale(ConvertDetails { from: 2, to: 1 }), 0.1); assert_eq!( - 10f64.normalize_scale(ConvertDetails { from: 1, to: 2 }), - 1.0 + 10.5f64.normalize_scale(ConvertDetails { from: 2, to: 1 }), + 1.05 ); - // 100 units with base 2 is 10 units with base 3 + // 100 units with base 3 is 10 units with base 2 assert_eq!( - 100f64.normalize_scale(ConvertDetails { from: 2, to: 3 }), + 100f64.normalize_scale(ConvertDetails { from: 3, to: 2 }), 10.0 ); - // 299 units with base 2 is 10 units with base 3 plus 99 leftovers + // 299 units with base 3 is 29.9 with base 2 assert_eq!( - 100f64.normalize_scale(ConvertDetails { from: 2, to: 3 }), - 10.0 + 299f64.normalize_scale(ConvertDetails { from: 3, to: 2 }), + 29.9 ); assert_eq!( - 999f64.normalize_scale(ConvertDetails { from: 6, to: 9 }), + 999f64.normalize_scale(ConvertDetails { from: 9, to: 6 }), 0.999 ); assert_eq!( - 1000f64.normalize_scale(ConvertDetails { from: 6, to: 9 }), + 1000f64.normalize_scale(ConvertDetails { from: 9, to: 6 }), 1.0 ); assert_eq!( - 1999f64.normalize_scale(ConvertDetails { from: 6, to: 9 }), + 1999f64.normalize_scale(ConvertDetails { from: 9, to: 6 }), 1.999 ); } From ca46d74617aafd4f778b183567ea0cea7c268344 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 6 Aug 2019 13:49:19 +0300 Subject: [PATCH 14/22] fix comment --- .../src/engines/ethereum_ledger/eth_engine.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs index 316773052..73c698ddf 100644 --- a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs +++ b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs @@ -769,7 +769,6 @@ where // If we receive a Quantity { amount: "1", scale: 9}, // we must normalize it to our engine's scale (typically 18 // for ETH/ERC20). - // Slightly counter-intuitive that `body.scale` is set as `to`. let amount = amount_from_connector.normalize_scale(ConvertDetails { from: body.scale, to: engine_scale, From 56237a6c621507eb52e41e909cde08b5b10bf317 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 6 Aug 2019 17:18:27 +0300 Subject: [PATCH 15/22] Revert "test(eth-xrp): Switch back to BTP" This reverts commit 36a3732ccd048383c2048f6630994de609a3df77. --- .../tests/eth_xrp_interoperable.rs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs index 2087d85d8..5bce391fa 100644 --- a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs +++ b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs @@ -201,11 +201,11 @@ fn eth_xrp_interoperable() { ilp_address: Address::from_str("example.bob.charlie").unwrap(), asset_code: "XRP".to_string(), asset_scale: xrp_decimals, - btp_incoming_token: Some("charlie".to_string()), + btp_incoming_token: None, btp_uri: None, - http_endpoint: None, - http_incoming_token: None, - http_outgoing_token: None, + http_endpoint: Some(format!("http://localhost:{}/ilp", node3_http)), + http_incoming_token: Some("charlie".to_string()), + http_outgoing_token: Some("bob".to_string()), max_packet_amount: u64::max_value(), min_balance: Some(-100), settle_threshold: Some(70000), @@ -265,9 +265,9 @@ fn eth_xrp_interoperable() { asset_scale: xrp_decimals, btp_incoming_token: None, btp_uri: None, - http_endpoint: None, + http_endpoint: Some(format!("http://localhost:{}/ilp", node3_http)), http_incoming_token: Some("in_charlie".to_string()), - http_outgoing_token: None, + http_outgoing_token: Some("out_charlie".to_string()), max_packet_amount: u64::max_value(), min_balance: None, settle_threshold: None, @@ -286,10 +286,10 @@ fn eth_xrp_interoperable() { asset_code: "XRP".to_string(), asset_scale: xrp_decimals, btp_incoming_token: None, - btp_uri: Some(format!("btp+ws://:charlie@localhost:{}", node2_btp)), - http_endpoint: None, - http_incoming_token: None, - http_outgoing_token: None, + btp_uri: None, + http_endpoint: Some(format!("http://localhost:{}/ilp", node2_http)), + http_incoming_token: Some("bob".to_string()), + http_outgoing_token: Some("charlie".to_string()), max_packet_amount: u64::max_value(), min_balance: Some(-100_000), settle_threshold: None, From 1f6f9a43b3b363c6313ffcce589699eb031c5732 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 6 Aug 2019 21:30:22 +0300 Subject: [PATCH 16/22] feat(settlement/engine): Return None when idempotent data not found instead of Error --- .../interledger-settlement-engines/src/api.rs | 24 +++++++++++-------- .../src/engines/ethereum_ledger/eth_engine.rs | 1 + .../engines/ethereum_ledger/test_helpers.rs | 6 ++--- .../src/stores/mod.rs | 3 +-- .../src/stores/redis_store_common.rs | 19 ++++++++------- crates/interledger-settlement/src/api.rs | 22 +++++++++++------ crates/interledger-settlement/src/lib.rs | 2 +- .../src/test_helpers.rs | 6 ++--- crates/interledger-store-redis/src/store.rs | 8 +++---- .../tests/settlement_test.rs | 10 +++++--- 10 files changed, 60 insertions(+), 41 deletions(-) diff --git a/crates/interledger-settlement-engines/src/api.rs b/crates/interledger-settlement-engines/src/api.rs index 59cbc627d..ad3f8acf4 100644 --- a/crates/interledger-settlement-engines/src/api.rs +++ b/crates/interledger-settlement-engines/src/api.rs @@ -7,7 +7,7 @@ use futures::{ use hyper::{Response, StatusCode}; use interledger_settlement::Quantity; use interledger_settlement::{IdempotentData, IdempotentStore}; -use log::trace; +use log::error; use ring::digest::{digest, SHA256}; use tokio::executor::spawn; use tower_web::{net::ConnectionStream, ServiceBuilder}; @@ -77,19 +77,23 @@ impl_web! { &self, idempotency_key: String, input_hash: [u8; 32], - ) -> impl Future { + ) -> impl Future, Error = String> { self.store .load_idempotent_data(idempotency_key.clone()) .map_err(move |_| { - let error_msg = "Couldn't load idempotent data".to_owned(); - trace!("{}", error_msg); + let error_msg = format!("Couldn't load idempotent data for idempotency key {:?}", idempotency_key); + error!("{}", error_msg); error_msg }) - .and_then(move |ret: IdempotentData| { - if ret.2 == input_hash { - Ok((ret.0, ret.1)) + .and_then(move |ret: Option| { + if let Some(ret) = ret { + if ret.2 == input_hash { + Ok(Some((ret.0, ret.1))) + } else { + Ok(Some((StatusCode::from_u16(409).unwrap(), Bytes::from(&b"Provided idempotency key is tied to other input"[..])))) + } } else { - Ok((StatusCode::from_u16(409).unwrap(), Bytes::from(&b"Provided idempotency key is tied to other input"[..]))) + Ok(None) } }) } @@ -104,8 +108,8 @@ impl_web! { Either::A( self.check_idempotency(idempotency_key.clone(), input_hash) .map_err(|err| Response::builder().status(502).body(err).unwrap()) - .then(move |ret: Result<(StatusCode, Bytes), Response>| { - if let Ok(ret) = ret { + .and_then(move |ret: Option<(StatusCode, Bytes)>| { + if let Some(ret) = ret { let resp = Response::builder().status(ret.0).body(String::from_utf8_lossy(&ret.1).to_string()).unwrap(); if ret.0.is_success() { return Either::A(Either::A(ok(resp))) diff --git a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs index 73c698ddf..d6ba86c05 100644 --- a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs +++ b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs @@ -769,6 +769,7 @@ where // If we receive a Quantity { amount: "1", scale: 9}, // we must normalize it to our engine's scale (typically 18 // for ETH/ERC20). + let amount = amount_from_connector.normalize_scale(ConvertDetails { from: body.scale, to: engine_scale, diff --git a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/test_helpers.rs b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/test_helpers.rs index b2e0fbe46..97f486a22 100644 --- a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/test_helpers.rs +++ b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/test_helpers.rs @@ -176,14 +176,14 @@ impl IdempotentStore for TestStore { fn load_idempotent_data( &self, idempotency_key: String, - ) -> Box + Send> { + ) -> Box, Error = ()> + Send> { let cache = self.cache.read(); if let Some(data) = cache.get(&idempotency_key) { let mut guard = self.cache_hits.write(); *guard += 1; // used to test how many times this branch gets executed - Box::new(ok((data.0, Bytes::from(data.1.clone()), data.2))) + Box::new(ok(Some((data.0, Bytes::from(data.1.clone()), data.2)))) } else { - Box::new(err(())) + Box::new(ok(None)) } } diff --git a/crates/interledger-settlement-engines/src/stores/mod.rs b/crates/interledger-settlement-engines/src/stores/mod.rs index 2274f08ba..26f97f009 100644 --- a/crates/interledger-settlement-engines/src/stores/mod.rs +++ b/crates/interledger-settlement-engines/src/stores/mod.rs @@ -13,11 +13,10 @@ pub type IdempotentEngineData = (StatusCode, Bytes, [u8; 32]); pub trait IdempotentEngineStore { /// Returns the API response that was saved when the idempotency key was used /// Also returns a hash of the input data which resulted in the response - /// Returns error if the data was not found fn load_idempotent_data( &self, idempotency_key: String, - ) -> Box + Send>; + ) -> Box, Error = ()> + Send>; /// Saves the data that was passed along with the api request for later /// The store MUST also save a hash of the input, so that it errors out on requests diff --git a/crates/interledger-settlement-engines/src/stores/redis_store_common.rs b/crates/interledger-settlement-engines/src/stores/redis_store_common.rs index 12c35d16b..c67bc8f3c 100644 --- a/crates/interledger-settlement-engines/src/stores/redis_store_common.rs +++ b/crates/interledger-settlement-engines/src/stores/redis_store_common.rs @@ -43,7 +43,7 @@ impl IdempotentEngineStore for EngineRedisStore { fn load_idempotent_data( &self, idempotency_key: String, - ) -> Box + Send> { + ) -> Box, Error = ()> + Send> { let idempotency_key_clone = idempotency_key.clone(); Box::new( cmd("HGETALL") @@ -65,13 +65,13 @@ impl IdempotentEngineStore for EngineRedisStore { trace!("Loaded idempotency key {:?} - {:?}", idempotency_key, ret); let mut input_hash: [u8; 32] = Default::default(); input_hash.copy_from_slice(input_hash_slice.as_ref()); - Ok(( + Ok(Some(( StatusCode::from_str(status_code).unwrap(), Bytes::from(data.clone()), input_hash, - )) + ))) } else { - Err(()) + Ok(None) } }, ), @@ -137,20 +137,23 @@ mod tests { .load_idempotent_data(IDEMPOTENCY_KEY.clone()) .map_err(|err| eprintln!("Redis error: {:?}", err)) .and_then(move |data1| { - assert_eq!(data1, (StatusCode::OK, Bytes::from("TEST"), input_hash)); + assert_eq!( + data1.unwrap(), + (StatusCode::OK, Bytes::from("TEST"), input_hash) + ); let _ = context; store .load_idempotent_data("asdf".to_string()) .map_err(|err| eprintln!("Redis error: {:?}", err)) - .and_then(move |_data2| { - assert_eq!(_data2.0, StatusCode::OK); + .and_then(move |data2| { + assert!(data2.is_none()); let _ = context; Ok(()) }) }) }) })) - .unwrap_err() // the second idempotent load fails + .unwrap() } } diff --git a/crates/interledger-settlement/src/api.rs b/crates/interledger-settlement/src/api.rs index d030f4b2f..c1fa8ff24 100644 --- a/crates/interledger-settlement/src/api.rs +++ b/crates/interledger-settlement/src/api.rs @@ -57,7 +57,7 @@ impl_web! { &self, idempotency_key: String, input_hash: [u8; 32], - ) -> impl Future { + ) -> impl Future, Error = String> { self.store .load_idempotent_data(idempotency_key.clone()) .map_err(move |_| { @@ -65,11 +65,15 @@ impl_web! { error!("{}", error_msg); error_msg }) - .and_then(move |ret: IdempotentData| { - if ret.2 == input_hash { - Ok((ret.0, ret.1)) + .and_then(move |ret: Option| { + if let Some(ret) = ret { + if ret.2 == input_hash { + Ok(Some((ret.0, ret.1))) + } else { + Ok(Some((StatusCode::from_u16(409).unwrap(), Bytes::from(&b"Provided idempotency key is tied to other input"[..])))) + } } else { - Ok((StatusCode::from_u16(409).unwrap(), Bytes::from(&b"Provided idempotency key is tied to other input"[..]))) + Ok(None) } }) } @@ -83,8 +87,12 @@ impl_web! { // key, perform the call and save the idempotent return data Either::A( self.check_idempotency(idempotency_key.clone(), input_hash) - .then(move |ret: Result<(StatusCode, Bytes), String>| { - if let Ok(ret) = ret { + .map_err(move |err| { + let status_code = StatusCode::from_u16(500).unwrap(); + Response::builder().status(status_code).body(err).unwrap() + }) + .and_then(move |ret: Option<(StatusCode, Bytes)>| { + if let Some(ret) = ret { if ret.0.is_success() { let resp = Response::builder().status(ret.0).body(ret.1).unwrap(); return Either::A(Either::A(ok(resp))) diff --git a/crates/interledger-settlement/src/lib.rs b/crates/interledger-settlement/src/lib.rs index 8d567dc8e..bb5a10334 100644 --- a/crates/interledger-settlement/src/lib.rs +++ b/crates/interledger-settlement/src/lib.rs @@ -81,7 +81,7 @@ pub trait IdempotentStore { fn load_idempotent_data( &self, idempotency_key: String, - ) -> Box + Send>; + ) -> Box, Error = ()> + Send>; /// Saves the data that was passed along with the api request for later /// The store MUST also save a hash of the input, so that it errors out on requests diff --git a/crates/interledger-settlement/src/test_helpers.rs b/crates/interledger-settlement/src/test_helpers.rs index 25c4a50d3..9eb195271 100644 --- a/crates/interledger-settlement/src/test_helpers.rs +++ b/crates/interledger-settlement/src/test_helpers.rs @@ -97,14 +97,14 @@ impl IdempotentStore for TestStore { fn load_idempotent_data( &self, idempotency_key: String, - ) -> Box + Send> { + ) -> Box, Error = ()> + Send> { let cache = self.cache.read(); if let Some(data) = cache.get(&idempotency_key) { let mut guard = self.cache_hits.write(); *guard += 1; // used to test how many times this branch gets executed - Box::new(ok((data.0, data.1.clone(), data.2))) + Box::new(ok(Some((data.0, data.1.clone(), data.2)))) } else { - Box::new(err(())) + Box::new(ok(None)) } } diff --git a/crates/interledger-store-redis/src/store.rs b/crates/interledger-store-redis/src/store.rs index 5ada796e6..d014b03bb 100644 --- a/crates/interledger-store-redis/src/store.rs +++ b/crates/interledger-store-redis/src/store.rs @@ -1083,7 +1083,7 @@ impl IdempotentStore for RedisStore { fn load_idempotent_data( &self, idempotency_key: String, - ) -> Box + Send> { + ) -> Box, Error = ()> + Send> { let idempotency_key_clone = idempotency_key.clone(); Box::new( cmd("HGETALL") @@ -1104,13 +1104,13 @@ impl IdempotentStore for RedisStore { trace!("Loaded idempotency key {:?} - {:?}", idempotency_key, ret); let mut input_hash: [u8; 32] = Default::default(); input_hash.copy_from_slice(input_hash_slice.as_ref()); - Ok(( + Ok(Some(( StatusCode::from_str(status_code).unwrap(), Bytes::from(data.clone()), input_hash, - )) + ))) } else { - Err(()) + Ok(None) } }), ) diff --git a/crates/interledger-store-redis/tests/settlement_test.rs b/crates/interledger-store-redis/tests/settlement_test.rs index 596de4d80..b7fbc0f24 100644 --- a/crates/interledger-store-redis/tests/settlement_test.rs +++ b/crates/interledger-store-redis/tests/settlement_test.rs @@ -53,20 +53,24 @@ fn saves_and_loads_idempotency_key_data_properly() { .load_idempotent_data(IDEMPOTENCY_KEY.clone()) .map_err(|err| eprintln!("Redis error: {:?}", err)) .and_then(move |data1| { - assert_eq!(data1, (StatusCode::OK, Bytes::from("TEST"), input_hash)); + assert_eq!( + data1.unwrap(), + (StatusCode::OK, Bytes::from("TEST"), input_hash) + ); let _ = context; store .load_idempotent_data("asdf".to_string()) .map_err(|err| eprintln!("Redis error: {:?}", err)) - .and_then(move |_data2| { + .and_then(move |data2| { + assert!(data2.is_none()); let _ = context; Ok(()) }) }) }) })) - .unwrap_err() // second idempotent load fails + .unwrap(); } #[test] From 9de2c7f94cf1ef6975baa23099f0726b8d84be17 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 6 Aug 2019 22:19:35 +0300 Subject: [PATCH 17/22] docs: fix readme in interoperability test --- examples/eth_xrp_three_nodes/README.md | 192 ++++++++++++------------- 1 file changed, 94 insertions(+), 98 deletions(-) diff --git a/examples/eth_xrp_three_nodes/README.md b/examples/eth_xrp_three_nodes/README.md index 3aa14af18..0a278c0ed 100755 --- a/examples/eth_xrp_three_nodes/README.md +++ b/examples/eth_xrp_three_nodes/README.md @@ -68,47 +68,47 @@ ganache-cli -m "abstract vacuum mammal awkward pudding scene penalty purchase di 1. Launch Alice's settlement engine in a new terminal by running: -```bash -RUST_LOG=interledger=debug $ILP settlement-engine ethereum-ledger \ ---key "380eb0f3d505f087e438eca80bc4df9a7faa24f868e69fc0440261a0fc0567dc" \ ---server_secret aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa \ ---confirmations 0 \ ---poll_frequency 1 \ ---ethereum_endpoint http://127.0.0.1:8545 \ ---connector_url http://127.0.0.1:7771 \ ---redis_uri redis://127.0.0.1:6379 \ ---asset_scale 6 \ ---watch_incoming true \ ---port 3000 -``` + ```bash + RUST_LOG=interledger=debug $ILP settlement-engine ethereum-ledger \ + --key "380eb0f3d505f087e438eca80bc4df9a7faa24f868e69fc0440261a0fc0567dc" \ + --server_secret aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa \ + --confirmations 0 \ + --poll_frequency 1 \ + --ethereum_endpoint http://127.0.0.1:8545 \ + --connector_url http://127.0.0.1:7771 \ + --redis_uri redis://127.0.0.1:6379 \ + --asset_scale 6 \ + --watch_incoming true \ + --port 3000 + ``` 1. Configure Alice's connector by putting the following data inside a config file, let's call that `alice.yml`. -```yaml -ilp_address: "example.alice" -secret_seed: "8852500887504328225458511465394229327394647958135038836332350604" -admin_auth_token: "hi_alice" -redis_connection: "redis://127.0.0.1:6379" -settlement_address: "127.0.0.1:7771" -http_address: "127.0.0.1:7770" -btp_address: "127.0.0.1:7768" -default_spsp_account: "0" -``` + ```yaml + ilp_address: "example.alice" + secret_seed: "8852500887504328225458511465394229327394647958135038836332350604" + admin_auth_token: "hi_alice" + redis_connection: "redis://127.0.0.1:6379" + settlement_address: "127.0.0.1:7771" + http_address: "127.0.0.1:7770" + btp_address: "127.0.0.1:7768" + default_spsp_account: "0" + ``` 1. Launch Alice's connector in a new terminal by running: -```bash -RUST_LOG="interledger=debug,interledger=trace" $ILP node --config alice.yml -``` + ```bash + RUST_LOG="interledger=debug,interledger=trace" $ILP node --config alice.yml + ``` 1. Insert Alice's account into her connector. -```bash -curl http://localhost:7770/accounts -X POST \ - -d "ilp_address=example.alice&asset_code=ETH&asset_scale=6&max_packet_amount=10&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=in_alice&outgoing_token=out_alice&settle_to=-10" \ - -H "Authorization: Bearer hi_alice" -``` + ```bash + curl http://localhost:7770/accounts -X POST \ + -d "ilp_address=example.alice&asset_code=ETH&asset_scale=6&max_packet_amount=10&http_endpoint=http://127.0.0.1:7770/ilp&http_incoming_token=in_alice&outgoing_token=out_alice&settle_to=-10" \ + -H "Authorization: Bearer hi_alice" + ``` 1. Insert Bob's account into Alice's connector. @@ -127,50 +127,50 @@ running. Bob. 1. Launch Bob's ETH settlement engine in a new terminal by running: -```bash -RUST_LOG=interledger=debug $ILP_ENGINE ethereum-ledger \ ---key "cc96601bc52293b53c4736a12af9130abf347669b3813f9ec4cafdf6991b087e" \ ---server_secret bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb \ ---confirmations 0 \ ---poll_frequency 1000 \ ---ethereum_endpoint http://127.0.0.1:8545 \ ---connector_url http://127.0.0.1:8771 \ ---redis_uri redis://127.0.0.1:6380 \ ---asset_scale 6 \ ---watch_incoming true \ ---port 3001 -``` + ```bash + RUST_LOG=interledger=debug $ILP_ENGINE ethereum-ledger \ + --key "cc96601bc52293b53c4736a12af9130abf347669b3813f9ec4cafdf6991b087e" \ + --server_secret bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb \ + --confirmations 0 \ + --poll_frequency 1000 \ + --ethereum_endpoint http://127.0.0.1:8545 \ + --connector_url http://127.0.0.1:8771 \ + --redis_uri redis://127.0.0.1:6380 \ + --asset_scale 6 \ + --watch_incoming true \ + --port 3001 + ``` 1. Launch Bob's XRP settlement engine in a new terminal by running: -```bash -DEBUG="xrp-settlement-engine" \ -LEDGER_ADDRESS="r3GDnYaYCk2XKzEDNYj59yMqDZ7zGih94K" \ -LEDGER_SECRET="ssnYUDNeNQrNij2EVJG6dDw258jA6" \ -CONNECTOR_URL="http://localhost:8771" \ -REDIS_PORT=6380 \ -ENGINE_PORT=3002 \ - ilp-settlement-xrp -``` + ```bash + DEBUG="xrp-settlement-engine" \ + LEDGER_ADDRESS="r3GDnYaYCk2XKzEDNYj59yMqDZ7zGih94K" \ + LEDGER_SECRET="ssnYUDNeNQrNij2EVJG6dDw258jA6" \ + CONNECTOR_URL="http://localhost:8771" \ + REDIS_PORT=6380 \ + ENGINE_PORT=3002 \ + ilp-settlement-xrp + ``` 1. Configure Bob's connector by putting the following data inside a config file, let's call that `bob.yml`. -```yaml -ilp_address: "example.bob" -secret_seed: "1604966725982139900555208458637022875563691455429373719368053354" -admin_auth_token: "hi_bob" -redis_connection: "redis://127.0.0.1:6380" -settlement_address: "127.0.0.1:8771" -http_address: "127.0.0.1:8770" -btp_address: "127.0.0.1:8768" -default_spsp_account: "0" -``` + ```yaml + ilp_address: "example.bob" + secret_seed: "1604966725982139900555208458637022875563691455429373719368053354" + admin_auth_token: "hi_bob" + redis_connection: "redis://127.0.0.1:6380" + settlement_address: "127.0.0.1:8771" + http_address: "127.0.0.1:8770" + btp_address: "127.0.0.1:8768" + default_spsp_account: "0" + ``` 1. Launch Bob's connector in a new terminal by running: -```bash -RUST_LOG="interledger=debug,interledger=trace" $ILP node --config bob.yml -``` + ```bash + RUST_LOG="interledger=debug,interledger=trace" $ILP node --config bob.yml + ``` 1. Insert Alice's account on Bob's connector (ETH Peer relation) @@ -194,49 +194,45 @@ RUST_LOG="interledger=debug,interledger=trace" $ILP node --config bob.yml Charlie. 1. Launch Charlie's XRP settlement engine in a new terminal by running: -```bash -DEBUG="xrp-settlement-engine" \ -LEDGER_ADDRESS="rGCUgMH4omQV1PUuYFoMAnA7esWFhE7ZEV" \ -LEDGER_SECRET="sahVoeg97nuitefnzL9GHjp2Z6kpj" \ -CONNECTOR_URL="http://localhost:9771" \ -REDIS_PORT=6381 \ -ENGINE_PORT=3003 \ - ilp-settlement-xrp &> $LOGS/xrp_engine_charlie.log & -``` + ```bash + DEBUG="xrp-settlement-engine" \ + LEDGER_ADDRESS="rGCUgMH4omQV1PUuYFoMAnA7esWFhE7ZEV" \ + LEDGER_SECRET="sahVoeg97nuitefnzL9GHjp2Z6kpj" \ + CONNECTOR_URL="http://localhost:9771" \ + REDIS_PORT=6381 \ + ENGINE_PORT=3003 \ + ilp-settlement-xrp &> $LOGS/xrp_engine_charlie.log & + ``` 1. Configure Charlie's connector by putting the following data inside a config file, let's call that `charlie.yml`. -```yaml -ilp_address: "example.bob.charlie" -secret_seed: "1232362131122139900555208458637022875563691455429373719368053354" -admin_auth_token: "hi_charlie" -redis_connection: "redis://127.0.0.1:6381" -settlement_address: "127.0.0.1:9771" -http_address: "127.0.0.1:9770" -btp_address: "127.0.0.1:9768" -default_spsp_account: "0" -``` + ```yaml + ilp_address: "example.bob.charlie" + secret_seed: "1232362131122139900555208458637022875563691455429373719368053354" + admin_auth_token: "hi_charlie" + redis_connection: "redis://127.0.0.1:6381" + settlement_address: "127.0.0.1:9771" + http_address: "127.0.0.1:9770" + btp_address: "127.0.0.1:9768" + default_spsp_account: "0" + ``` 1. Launch Charlie's connector in a new terminal by running: -```bash -RUST_LOG="interledger=debug,interledger=trace" $ILP node --config charlie.yml -``` + ```bash + RUST_LOG="interledger=debug,interledger=trace" $ILP node --config charlie.yml + ``` 1. Insert Charlie's account details on Charlie's connector -```bash -curl http://localhost:9770/accounts -X POST \ - -d "ilp_address=example.bob.charlie&asset_code=XRP&asset_scale=6&max_packet_amount=10&http_endpoint=http://127.0.0.1:9770/ilp&http_incoming_token=in_charlie&outgoing_token=out_charlie&settle_to=-10" \ - -H "Authorization: Bearer hi_charlie" -``` + + ```bash + curl http://localhost:9770/accounts -X POST \ + -d "ilp_address=example.bob.charlie&asset_code=XRP&asset_scale=6&max_packet_amount=10&http_endpoint=http://127.0.0.1:9770/ilp&http_incoming_token=in_charlie&outgoing_token=out_charlie&settle_to=-10" \ + -H "Authorization: Bearer hi_charlie" + ``` 1. Insert Bob's account details on Charlie's connector (XRP Parent relation) -```bash -curl http://localhost:9770/accounts -X POST \ - -d "ilp_address=example.bob&asset_code=XRP&asset_scale=6&max_packet_amount=10&settlement_engine_url=http://127.0.0.1:3003&settlement_engine_asset_scale=6&http_endpoint=http://127.0.0.1:8770/ilp&http_incoming_token=bob&http_outgoing_token=charlie&settle_threshold=70&min_balance=-100&settle_to=10&routing_relation=Parent&receive_routes=true&send_routes=false" \ - -H "Authorization: Bearer hi_charlie" & -``` ```bash curl http://localhost:9770/accounts -X POST \ From b5b62ecea848e3a4e0ea092925e1630fb7749637 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Thu, 8 Aug 2019 11:07:04 +0300 Subject: [PATCH 18/22] fix(settlement): Make u64 conversions overflow-safe, and warn for f64 conversions --- .../src/exchange_rates_service.rs | 4 +++ crates/interledger-settlement/src/api.rs | 7 ++--- crates/interledger-settlement/src/lib.rs | 27 ++++++++++++++++--- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/crates/interledger-service-util/src/exchange_rates_service.rs b/crates/interledger-service-util/src/exchange_rates_service.rs index f3774b2b2..1e1e3684f 100644 --- a/crates/interledger-service-util/src/exchange_rates_service.rs +++ b/crates/interledger-service-util/src/exchange_rates_service.rs @@ -90,6 +90,10 @@ where .build())); }; + // Note that this is not an overflow safe operation. Given realistic + // assumptions, the asset scale will be <=18 and >=1. + // It is doubtful that the exchange rate between two assets, + // multiplied by 18 would exceed std::f64::MAX let scaled_rate = rate.normalize_scale(ConvertDetails { from: request.from.asset_scale(), to: request.to.asset_scale(), diff --git a/crates/interledger-settlement/src/api.rs b/crates/interledger-settlement/src/api.rs index c1fa8ff24..31e14853d 100644 --- a/crates/interledger-settlement/src/api.rs +++ b/crates/interledger-settlement/src/api.rs @@ -11,7 +11,7 @@ use hyper::{Response, StatusCode}; use interledger_ildcp::IldcpAccount; use interledger_packet::PrepareBuilder; use interledger_service::{AccountStore, OutgoingRequest, OutgoingService}; -use log::error; +use log::{debug, error}; use num_bigint::BigUint; use num_traits::cast::ToPrimitive; use ring::digest::{digest, SHA256}; @@ -191,9 +191,10 @@ impl_web! { to: account.asset_scale(), }); // If we'd overflow, settle for the maximum u64 value - let safe_amount = if let Some(amount) = amount.to_u64() { - amount + let safe_amount = if let Some(amount_u64) = amount.to_u64() { + amount_u64 } else { + debug!("Amount settled from engine overflowed during conversion to connector scale: {}. Settling for u64::MAX", amount); std::u64::MAX }; store.update_balance_for_incoming_settlement(account_id, safe_amount, idempotency_key) diff --git a/crates/interledger-settlement/src/lib.rs b/crates/interledger-settlement/src/lib.rs index bb5a10334..c092a9302 100644 --- a/crates/interledger-settlement/src/lib.rs +++ b/crates/interledger-settlement/src/lib.rs @@ -19,7 +19,9 @@ mod fixtures; mod message_service; #[cfg(test)] mod test_helpers; +use log::debug; use num_bigint::BigUint; +use num_traits::cast::ToPrimitive; use std::ops::{Div, Mul}; pub use api::SettlementApi; @@ -94,6 +96,7 @@ pub trait IdempotentStore { ) -> Box + Send>; } +#[derive(Debug)] pub struct ConvertDetails { pub from: u8, pub to: u8, @@ -108,15 +111,27 @@ impl Convert for u64 { fn normalize_scale(&self, details: ConvertDetails) -> Self { let scale_diff = (details.from as i8 - details.to as i8).abs() as u8; let scale = 10u64.pow(scale_diff.into()); - if details.to >= details.from { - self * scale + let num = BigUint::from(*self); + let num = if details.to >= details.from { + num.mul(scale) } else { - self / scale + num.div(scale) + }; + if let Some(num_u64) = num.to_u64() { + num_u64 + } else { + debug!( + "Overflow during conversion from {} {:?}. Using u64::MAX", + num, details + ); + std::u64::MAX } } } impl Convert for f64 { + // Not overflow safe. Would require using a package for Big floating point + // numbers such as BigDecimal fn normalize_scale(&self, details: ConvertDetails) -> Self { let scale_diff = (details.from as i8 - details.to as i8).abs() as u8; let scale = 10f64.powi(scale_diff.into()); @@ -147,6 +162,12 @@ mod tests { #[test] fn u64_test() { + // does not overflow + let huge_number = std::u64::MAX / 10; + assert_eq!( + huge_number.normalize_scale(ConvertDetails { from: 1, to: 18 }), + std::u64::MAX + ); // 1 unit with scale 1, is 1 unit with scale 1 assert_eq!(1u64.normalize_scale(ConvertDetails { from: 1, to: 1 }), 1); // there's leftovers for all number slots which do not increase in From 1c3a182d39c358cf784c7f376bdf5f580285692d Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Thu, 8 Aug 2019 11:18:22 +0300 Subject: [PATCH 19/22] docs: fix review comments --- .../src/engines/ethereum_ledger/eth_engine.rs | 4 +--- .../tests/eth_xrp_interoperable.rs | 6 +++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs index d6ba86c05..b86a4c41f 100644 --- a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs +++ b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs @@ -767,9 +767,7 @@ where })) .and_then(move |amount_from_connector| { // If we receive a Quantity { amount: "1", scale: 9}, - // we must normalize it to our engine's scale (typically 18 - // for ETH/ERC20). - + // we must normalize it to our engine's scale let amount = amount_from_connector.normalize_scale(ConvertDetails { from: body.scale, to: engine_scale, diff --git a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs index 5bce391fa..b85f984e9 100644 --- a/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs +++ b/crates/interledger-settlement-engines/tests/eth_xrp_interoperable.rs @@ -122,7 +122,7 @@ fn eth_xrp_interoperable() { max_packet_amount: u64::max_value(), min_balance: None, settle_threshold: None, - settle_to: Some(-10), + settle_to: None, send_routes: false, receive_routes: false, routing_relation: None, @@ -343,7 +343,7 @@ fn eth_xrp_interoperable() { assert_eq!(balances[0], -71000); // Since Alice has configured Bob's // `settle_threshold` and `settle_to` to be - // 70k and -10k respectively, once she + // 70k and 10k respectively, once she // exceeded the 70k threshold, she made a 61k // Gwei settlement to Bob so that their debt // settles down to 10k. @@ -362,7 +362,7 @@ fn eth_xrp_interoperable() { // which is 5k (71k - 5k = 66k). assert_eq!(balances[3], 5000); // Charlie's balance indicates that he's - // received 71k drops from Alice + // received 71k drops (the total amount Alice sent him) assert_eq!(balances[4], 71000); // And he sees is owed 5k by Bob. assert_eq!(balances[5], -5000); From 449e4a6da33a22e286985f98dff99e65d2ebb909 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Thu, 8 Aug 2019 16:48:12 +0300 Subject: [PATCH 20/22] improvement(settlement): return Error on overflow during conversion - If exchange rate normalization fails return a reject packet - In all other places we use BigUint so the conversion should never fail, but we still handle the error graciously - prefer shadwing of variable name to avoid using the wrong variable - clarify comment about SettlementEngineDetails --- .../src/exchange_rates_service.rs | 29 +++- .../src/engines/ethereum_ledger/eth_engine.rs | 24 ++- crates/interledger-settlement/src/api.rs | 34 ++-- crates/interledger-settlement/src/lib.rs | 159 +++++++++++++----- 4 files changed, 179 insertions(+), 67 deletions(-) diff --git a/crates/interledger-service-util/src/exchange_rates_service.rs b/crates/interledger-service-util/src/exchange_rates_service.rs index 1e1e3684f..e97f9b4e2 100644 --- a/crates/interledger-service-util/src/exchange_rates_service.rs +++ b/crates/interledger-service-util/src/exchange_rates_service.rs @@ -98,9 +98,32 @@ where from: request.from.asset_scale(), to: request.to.asset_scale(), }); - let outgoing_amount = (request.prepare.amount() as f64) * scaled_rate; - request.prepare.set_amount(outgoing_amount as u64); - trace!("Converted incoming amount of: {} {} (scale {}) from account {} to outgoing amount of: {} {} (scale {}) for account {}", request.original_amount, request.from.asset_code(), request.from.asset_scale(), request.from.id(), outgoing_amount, request.to.asset_code(), request.to.asset_scale(), request.to.id()); + + match scaled_rate { + Ok(scaled_rate) => { + let outgoing_amount = (request.prepare.amount() as f64) * scaled_rate; + request.prepare.set_amount(outgoing_amount as u64); + trace!("Converted incoming amount of: {} {} (scale {}) from account {} to outgoing amount of: {} {} (scale {}) for account {}", + request.original_amount, request.from.asset_code(), request.from.asset_scale(), request.from.id(), + outgoing_amount, request.to.asset_code(), request.to.asset_scale(), request.to.id()); + } + Err(_) => { + return Box::new(err(RejectBuilder { + code: ErrorCode::F02_UNREACHABLE, + message: format!( + "Could not convert exchange rate from {}:{} to: {}:{}", + request.from.asset_code(), + request.from.asset_scale(), + request.to.asset_code(), + request.to.asset_scale(), + ) + .as_bytes(), + triggered_by: Some(&self.ilp_address), + data: &[], + } + .build())); + } + } } Box::new(self.next.send_request(request)) diff --git a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs index b86a4c41f..0932fecaa 100644 --- a/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs +++ b/crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs @@ -761,7 +761,7 @@ where let engine_scale = self.asset_scale; Box::new( result(BigUint::from_str(&body.amount).map_err(move |err| { - let error_msg = format!("Error converting to U256 {:?}", err); + let error_msg = format!("Error converting to BigUint {:?}", err); error!("{:?}", error_msg); (StatusCode::from_u16(400).unwrap(), error_msg) })) @@ -773,13 +773,21 @@ where to: engine_scale, }); - // Typecast from num_bigint::BigUInt because we're using - // ethereum_types::U256 for all rust-web3 related functionality - result(U256::from_dec_str(&amount.to_string()).map_err(move |err| { - let error_msg = format!("Error converting to U256 {:?}", err); - error!("{:?}", error_msg); - (StatusCode::from_u16(400).unwrap(), error_msg) - })) + result(amount) + .map_err(move |err| { + let error_msg = format!("Error scaling amount: {:?}", err); + error!("{:?}", error_msg); + (StatusCode::from_u16(400).unwrap(), error_msg) + }) + .and_then(move |amount| { + // Typecast from num_bigint::BigUInt because we're using + // ethereum_types::U256 for all rust-web3 related functionality + result(U256::from_dec_str(&amount.to_string()).map_err(move |err| { + let error_msg = format!("Error converting to U256 {:?}", err); + error!("{:?}", error_msg); + (StatusCode::from_u16(400).unwrap(), error_msg) + })) + }) }) .and_then(move |amount| { self_clone diff --git a/crates/interledger-settlement/src/api.rs b/crates/interledger-settlement/src/api.rs index 31e14853d..b9df66d1e 100644 --- a/crates/interledger-settlement/src/api.rs +++ b/crates/interledger-settlement/src/api.rs @@ -190,23 +190,31 @@ impl_web! { from: engine_scale, to: account.asset_scale(), }); - // If we'd overflow, settle for the maximum u64 value - let safe_amount = if let Some(amount_u64) = amount.to_u64() { - amount_u64 - } else { - debug!("Amount settled from engine overflowed during conversion to connector scale: {}. Settling for u64::MAX", amount); - std::u64::MAX - }; - store.update_balance_for_incoming_settlement(account_id, safe_amount, idempotency_key) + result(amount.clone()) .map_err(move |_| { - let error_msg = format!("Error updating balance of account: {} for incoming settlement of amount: {}", account_id, amount); + let error_msg = format!("Could not convert amount: {:?}", amount); error!("{}", error_msg); (StatusCode::from_u16(500).unwrap(), error_msg) }) - .and_then(move |_| { - // TODO: Return a Quantity of safe_amount and account.asset_scale() - let ret = Bytes::from("Success"); - Ok((StatusCode::OK, ret)) + .and_then(move |amount| { + // If we'd overflow, settle for the maximum u64 value + let amount = if let Some(amount_u64) = amount.to_u64() { + amount_u64 + } else { + debug!("Amount settled from engine overflowed during conversion to connector scale: {:?}. Settling for u64::MAX", amount); + std::u64::MAX + }; + store.update_balance_for_incoming_settlement(account_id, amount, idempotency_key) + .map_err(move |_| { + let error_msg = format!("Error updating balance of account: {} for incoming settlement of amount: {}", account_id, amount); + error!("{}", error_msg); + (StatusCode::from_u16(500).unwrap(), error_msg) + }) + .and_then(move |_| { + // TODO: Return a Quantity of amount and account.asset_scale() + let ret = Bytes::from("Success"); + Ok((StatusCode::OK, ret)) + }) }) }) })) diff --git a/crates/interledger-settlement/src/lib.rs b/crates/interledger-settlement/src/lib.rs index c092a9302..4a9e3f19b 100644 --- a/crates/interledger-settlement/src/lib.rs +++ b/crates/interledger-settlement/src/lib.rs @@ -19,9 +19,7 @@ mod fixtures; mod message_service; #[cfg(test)] mod test_helpers; -use log::debug; use num_bigint::BigUint; -use num_traits::cast::ToPrimitive; use std::ops::{Div, Mul}; pub use api::SettlementApi; @@ -47,6 +45,10 @@ impl Quantity { } } +// TODO: Since we still haven't finalized all the settlement details, we might +// end up deciding to add some more values, e.g. some settlement engine uid or similar. +// All instances of this struct should be replaced with Url instances once/if we +// agree that there is no more info required to refer to an engine. pub struct SettlementEngineDetails { /// Base URL of the settlement engine pub url: Url, @@ -104,53 +106,61 @@ pub struct ConvertDetails { /// Traits for u64 and f64 asset code conversions for amounts and rates pub trait Convert { - fn normalize_scale(&self, details: ConvertDetails) -> Self; + type Item: Sized; + + // Returns the scaled result, or an error if there was an overflow + fn normalize_scale(&self, details: ConvertDetails) -> Result; } impl Convert for u64 { - fn normalize_scale(&self, details: ConvertDetails) -> Self { + type Item = u64; + + fn normalize_scale(&self, details: ConvertDetails) -> Result { let scale_diff = (details.from as i8 - details.to as i8).abs() as u8; let scale = 10u64.pow(scale_diff.into()); - let num = BigUint::from(*self); - let num = if details.to >= details.from { - num.mul(scale) + let (res, overflow) = if details.to >= details.from { + self.overflowing_mul(scale) } else { - num.div(scale) + self.overflowing_div(scale) }; - if let Some(num_u64) = num.to_u64() { - num_u64 + if overflow { + Err(()) } else { - debug!( - "Overflow during conversion from {} {:?}. Using u64::MAX", - num, details - ); - std::u64::MAX + Ok(res) } } } impl Convert for f64 { + type Item = f64; // Not overflow safe. Would require using a package for Big floating point // numbers such as BigDecimal - fn normalize_scale(&self, details: ConvertDetails) -> Self { + fn normalize_scale(&self, details: ConvertDetails) -> Result { let scale_diff = (details.from as i8 - details.to as i8).abs() as u8; let scale = 10f64.powi(scale_diff.into()); - if details.to >= details.from { + let res = if details.to >= details.from { self * scale } else { self / scale + }; + if res == std::f64::INFINITY { + Err(()) + } else { + Ok(res) } } } impl Convert for BigUint { - fn normalize_scale(&self, details: ConvertDetails) -> Self { + type Item = BigUint; + + fn normalize_scale(&self, details: ConvertDetails) -> Result { let scale_diff = (details.from as i8 - details.to as i8).abs() as u8; let scale = 10u64.pow(scale_diff.into()); if details.to >= details.from { - self.mul(scale) + Ok(self.mul(scale)) } else { - self.div(scale) + Ok(self.div(scale)) } } } @@ -162,46 +172,76 @@ mod tests { #[test] fn u64_test() { - // does not overflow + // overflows let huge_number = std::u64::MAX / 10; assert_eq!( - huge_number.normalize_scale(ConvertDetails { from: 1, to: 18 }), - std::u64::MAX + huge_number + .normalize_scale(ConvertDetails { from: 1, to: 18 }) + .unwrap_err(), + (), ); // 1 unit with scale 1, is 1 unit with scale 1 - assert_eq!(1u64.normalize_scale(ConvertDetails { from: 1, to: 1 }), 1); + assert_eq!( + 1u64.normalize_scale(ConvertDetails { from: 1, to: 1 }) + .unwrap(), + 1 + ); // there's leftovers for all number slots which do not increase in // increments of 10^abs(to_scale-from_scale) - assert_eq!(1u64.normalize_scale(ConvertDetails { from: 2, to: 1 }), 0); + assert_eq!( + 1u64.normalize_scale(ConvertDetails { from: 2, to: 1 }) + .unwrap(), + 0 + ); // 1 unit with scale 1, is 10 units with scale 2 - assert_eq!(1u64.normalize_scale(ConvertDetails { from: 1, to: 2 }), 10); + assert_eq!( + 1u64.normalize_scale(ConvertDetails { from: 1, to: 2 }) + .unwrap(), + 10 + ); // 1 gwei (scale 9) is 1e9 wei (scale 18) assert_eq!( - 1u64.normalize_scale(ConvertDetails { from: 9, to: 18 }), + 1u64.normalize_scale(ConvertDetails { from: 9, to: 18 }) + .unwrap(), 1_000_000_000 ); // 1_000_000_000 wei is 1gwei assert_eq!( - 1_000_000_000u64.normalize_scale(ConvertDetails { from: 18, to: 9 }), + 1_000_000_000u64 + .normalize_scale(ConvertDetails { from: 18, to: 9 }) + .unwrap(), 1, ); // 10 units with base 2 is 100 units with base 3 assert_eq!( - 10u64.normalize_scale(ConvertDetails { from: 2, to: 3 }), + 10u64 + .normalize_scale(ConvertDetails { from: 2, to: 3 }) + .unwrap(), 100 ); // 299 units with base 3 is 29 units with base 2 (0.9 leftovers) assert_eq!( - 299u64.normalize_scale(ConvertDetails { from: 3, to: 2 }), + 299u64 + .normalize_scale(ConvertDetails { from: 3, to: 2 }) + .unwrap(), 29 ); - assert_eq!(999u64.normalize_scale(ConvertDetails { from: 9, to: 6 }), 0); assert_eq!( - 1000u64.normalize_scale(ConvertDetails { from: 9, to: 6 }), + 999u64 + .normalize_scale(ConvertDetails { from: 9, to: 6 }) + .unwrap(), + 0 + ); + assert_eq!( + 1000u64 + .normalize_scale(ConvertDetails { from: 9, to: 6 }) + .unwrap(), 1 ); assert_eq!( - 1999u64.normalize_scale(ConvertDetails { from: 9, to: 6 }), + 1999u64 + .normalize_scale(ConvertDetails { from: 9, to: 6 }) + .unwrap(), 1 ); } @@ -209,46 +249,79 @@ mod tests { #[allow(clippy::float_cmp)] #[test] fn f64_test() { + // overflow + assert_eq!( + std::f64::MAX + .normalize_scale(ConvertDetails { + from: 1, + to: std::u8::MAX, + }) + .unwrap_err(), + () + ); + // 1 unit with base 1, is 1 unit with base 1 - assert_eq!(1f64.normalize_scale(ConvertDetails { from: 1, to: 1 }), 1.0); + assert_eq!( + 1f64.normalize_scale(ConvertDetails { from: 1, to: 1 }) + .unwrap(), + 1.0 + ); // 1 unit with base 10, is 10 units with base 1 assert_eq!( - 1f64.normalize_scale(ConvertDetails { from: 1, to: 2 }), + 1f64.normalize_scale(ConvertDetails { from: 1, to: 2 }) + .unwrap(), 10.0 ); // 1 sat is 1e9 wei (multiplied by rate) assert_eq!( - 1f64.normalize_scale(ConvertDetails { from: 9, to: 18 }), + 1f64.normalize_scale(ConvertDetails { from: 9, to: 18 }) + .unwrap(), 1_000_000_000.0 ); // 1.0 unit with base 2 is 0.1 unit with base 1 - assert_eq!(1f64.normalize_scale(ConvertDetails { from: 2, to: 1 }), 0.1); assert_eq!( - 10.5f64.normalize_scale(ConvertDetails { from: 2, to: 1 }), + 1f64.normalize_scale(ConvertDetails { from: 2, to: 1 }) + .unwrap(), + 0.1 + ); + assert_eq!( + 10.5f64 + .normalize_scale(ConvertDetails { from: 2, to: 1 }) + .unwrap(), 1.05 ); // 100 units with base 3 is 10 units with base 2 assert_eq!( - 100f64.normalize_scale(ConvertDetails { from: 3, to: 2 }), + 100f64 + .normalize_scale(ConvertDetails { from: 3, to: 2 }) + .unwrap(), 10.0 ); // 299 units with base 3 is 29.9 with base 2 assert_eq!( - 299f64.normalize_scale(ConvertDetails { from: 3, to: 2 }), + 299f64 + .normalize_scale(ConvertDetails { from: 3, to: 2 }) + .unwrap(), 29.9 ); assert_eq!( - 999f64.normalize_scale(ConvertDetails { from: 9, to: 6 }), + 999f64 + .normalize_scale(ConvertDetails { from: 9, to: 6 }) + .unwrap(), 0.999 ); assert_eq!( - 1000f64.normalize_scale(ConvertDetails { from: 9, to: 6 }), + 1000f64 + .normalize_scale(ConvertDetails { from: 9, to: 6 }) + .unwrap(), 1.0 ); assert_eq!( - 1999f64.normalize_scale(ConvertDetails { from: 9, to: 6 }), + 1999f64 + .normalize_scale(ConvertDetails { from: 9, to: 6 }) + .unwrap(), 1.999 ); } From d227963267413d2d888c8191101d5cbe2cd50aea Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Thu, 8 Aug 2019 18:31:03 +0300 Subject: [PATCH 21/22] improvement(exchange-rate): make the scale conversion after the exchange rate is applied --- .../src/exchange_rates_service.rs | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/crates/interledger-service-util/src/exchange_rates_service.rs b/crates/interledger-service-util/src/exchange_rates_service.rs index e97f9b4e2..2fc46d2fa 100644 --- a/crates/interledger-service-util/src/exchange_rates_service.rs +++ b/crates/interledger-service-util/src/exchange_rates_service.rs @@ -90,18 +90,29 @@ where .build())); }; - // Note that this is not an overflow safe operation. Given realistic - // assumptions, the asset scale will be <=18 and >=1. - // It is doubtful that the exchange rate between two assets, - // multiplied by 18 would exceed std::f64::MAX - let scaled_rate = rate.normalize_scale(ConvertDetails { + // Can we overflow here? + let outgoing_amount = (request.prepare.amount() as f64) * rate; + let outgoing_amount = outgoing_amount.normalize_scale(ConvertDetails { from: request.from.asset_scale(), to: request.to.asset_scale(), }); - match scaled_rate { - Ok(scaled_rate) => { - let outgoing_amount = (request.prepare.amount() as f64) * scaled_rate; + match outgoing_amount { + Ok(outgoing_amount) => { + // f64 which cannot fit in u64 gets cast as 0 + if outgoing_amount != 0.0 && outgoing_amount as u64 == 0 { + return Box::new(err(RejectBuilder { + code: ErrorCode::F08_AMOUNT_TOO_LARGE, + message: format!( + "Could not cast outgoing amount to u64 {}", + outgoing_amount, + ) + .as_bytes(), + triggered_by: Some(&self.ilp_address), + data: &[], + } + .build())); + } request.prepare.set_amount(outgoing_amount as u64); trace!("Converted incoming amount of: {} {} (scale {}) from account {} to outgoing amount of: {} {} (scale {}) for account {}", request.original_amount, request.from.asset_code(), request.from.asset_scale(), request.from.id(), @@ -109,13 +120,14 @@ where } Err(_) => { return Box::new(err(RejectBuilder { - code: ErrorCode::F02_UNREACHABLE, + code: ErrorCode::F08_AMOUNT_TOO_LARGE, message: format!( - "Could not convert exchange rate from {}:{} to: {}:{}", + "Could not convert exchange rate from {}:{} to: {}:{}. Got incoming amount: {}", request.from.asset_code(), request.from.asset_scale(), request.to.asset_code(), request.to.asset_scale(), + request.prepare.amount(), ) .as_bytes(), triggered_by: Some(&self.ilp_address), From fa69cd685b57a9a8170fcb904f0ffff1f4884958 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Fri, 9 Aug 2019 09:57:00 +0300 Subject: [PATCH 22/22] test(exchange-rate): Add tests which verify that rate conversions fail correctly --- .../src/exchange_rates_service.rs | 165 +++++++++++++++++- 1 file changed, 164 insertions(+), 1 deletion(-) diff --git a/crates/interledger-service-util/src/exchange_rates_service.rs b/crates/interledger-service-util/src/exchange_rates_service.rs index 2fc46d2fa..77553f9b8 100644 --- a/crates/interledger-service-util/src/exchange_rates_service.rs +++ b/crates/interledger-service-util/src/exchange_rates_service.rs @@ -99,7 +99,9 @@ where match outgoing_amount { Ok(outgoing_amount) => { - // f64 which cannot fit in u64 gets cast as 0 + // The conversion succeeded, but the produced f64 + // is larger than the maximum value for a u64. + // When it gets cast to a u64, it will end up being 0. if outgoing_amount != 0.0 && outgoing_amount as u64 == 0 { return Box::new(err(RejectBuilder { code: ErrorCode::F08_AMOUNT_TOO_LARGE, @@ -119,6 +121,10 @@ where outgoing_amount, request.to.asset_code(), request.to.asset_scale(), request.to.id()); } Err(_) => { + // This branch gets executed when the `Convert` trait + // returns an error. Happens due to float + // multiplication overflow . + // (float overflow in Rust produces +inf) return Box::new(err(RejectBuilder { code: ErrorCode::F08_AMOUNT_TOO_LARGE, message: format!( @@ -141,3 +147,160 @@ where Box::new(self.next.send_request(request)) } } + +#[cfg(test)] +mod tests { + use super::*; + use futures::{future::ok, Future}; + use interledger_ildcp::IldcpAccount; + use interledger_packet::{Address, FulfillBuilder, PrepareBuilder}; + use interledger_service::{outgoing_service_fn, Account}; + use std::collections::HashMap; + use std::str::FromStr; + use std::{ + sync::{Arc, Mutex}, + time::SystemTime, + }; + + #[test] + fn exchange_rate_ok() { + let ret = exchange_rate(100, 1, 1.0, 1, 2.0); + assert_eq!(ret.1[0].prepare.amount(), 200); + + let ret = exchange_rate(1_000_000, 1, 3.0, 1, 2.0); + assert_eq!(ret.1[0].prepare.amount(), 666_666); + } + + #[test] + fn exchange_conversion_error() { + // rejects f64 that does not fit in u64 + let ret = exchange_rate(std::u64::MAX, 1, 1.0, 1, 2.0); + let reject = ret.0.unwrap_err(); + assert_eq!(reject.code(), ErrorCode::F08_AMOUNT_TOO_LARGE); + assert!(reject.message().starts_with(b"Could not cast")); + + // `Convert` errored + let ret = exchange_rate(std::u64::MAX, 1, 1.0, 255, std::f64::MAX); + let reject = ret.0.unwrap_err(); + assert_eq!(reject.code(), ErrorCode::F08_AMOUNT_TOO_LARGE); + assert!(reject.message().starts_with(b"Could not convert")); + } + + // Instantiates an exchange rate service and returns the fulfill/reject + // packet and the outgoing request after performing an asset conversion + fn exchange_rate( + amount: u64, + scale1: u8, + rate1: f64, + scale2: u8, + rate2: f64, + ) -> (Result, Vec>) { + let requests = Arc::new(Mutex::new(Vec::new())); + let requests_clone = requests.clone(); + let outgoing = outgoing_service_fn(move |request| { + requests_clone.lock().unwrap().push(request); + Box::new(ok(FulfillBuilder { + fulfillment: &[0; 32], + data: b"hello!", + } + .build())) + }); + let mut service = test_service(rate1, rate2, outgoing); + let result = service + .send_request(OutgoingRequest { + from: TestAccount::new("ABC".to_owned(), scale1), + to: TestAccount::new("XYZ".to_owned(), scale2), + original_amount: amount, + prepare: PrepareBuilder { + destination: Address::from_str("example.destination").unwrap(), + amount, + expires_at: SystemTime::now(), + execution_condition: &[1; 32], + data: b"hello", + } + .build(), + }) + .wait(); + + let reqs = requests.lock().unwrap(); + (result, reqs.clone()) + } + + #[derive(Debug, Clone)] + struct TestAccount { + ilp_address: Address, + asset_code: String, + asset_scale: u8, + } + impl TestAccount { + fn new(asset_code: String, asset_scale: u8) -> Self { + TestAccount { + ilp_address: Address::from_str("example.alice").unwrap(), + asset_code, + asset_scale, + } + } + } + + impl Account for TestAccount { + type AccountId = u64; + + fn id(&self) -> u64 { + 0 + } + } + + impl IldcpAccount for TestAccount { + fn asset_code(&self) -> &str { + &self.asset_code + } + + fn asset_scale(&self) -> u8 { + self.asset_scale + } + + fn client_address(&self) -> &Address { + &self.ilp_address + } + } + + #[derive(Debug, Clone)] + struct TestStore { + rates: HashMap, (f64, f64)>, + } + + impl ExchangeRateStore for TestStore { + fn get_exchange_rates(&self, asset_codes: &[&str]) -> Result, ()> { + let mut ret = Vec::new(); + let key = vec![asset_codes[0].to_owned(), asset_codes[1].to_owned()]; + let v = self.rates.get(&key); + if let Some(v) = v { + ret.push(v.0); + ret.push(v.1); + } else { + return Err(()); + } + Ok(ret) + } + } + + fn test_store(rate1: f64, rate2: f64) -> TestStore { + let mut rates = HashMap::new(); + rates.insert(vec!["ABC".to_owned(), "XYZ".to_owned()], (rate1, rate2)); + TestStore { rates } + } + + fn test_service( + rate1: f64, + rate2: f64, + handler: impl OutgoingService + Clone + Send + Sync, + ) -> ExchangeRateService< + TestStore, + impl OutgoingService + Clone + Send + Sync, + TestAccount, + > { + let store = test_store(rate1, rate2); + ExchangeRateService::new(Address::from_str("example.bob").unwrap(), store, handler) + } + +}