Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Batch Payments Feature #155

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
245 changes: 80 additions & 165 deletions src/client/batch.rs
Original file line number Diff line number Diff line change
@@ -1,191 +1,106 @@
use serde::Deserialize;

use std::fs::File;

use super::*;

use csv;
use rust_decimal::Decimal;
use rust_decimal_macros::dec;
use std::path::Path;
use std::str::FromStr;

#[derive(Debug, Deserialize)]
pub struct PaymentInput {
pub username: String,
pub usd: Decimal,
pub memo: Option<String>,
}
use crate::client::GaloyClient;
use crate::client::Wallet;

impl From<PaymentInput> for Payment {
fn from(input: PaymentInput) -> Payment {
Payment {
username: input.username,
usd: input.usd,
sats: None,
wallet_id: None,
memo: input.memo,
}
// Utility function to check if file exists
pub fn check_file_exists(file: &str) -> anyhow::Result<()> {
let file_path = Path::new(&file);
if !file_path.exists() {
return Err(anyhow::anyhow!("File not found: {}", file));
}
Ok(())
}

#[derive(Debug)]
struct Payment {
username: String,
usd: Decimal,
sats: Option<Decimal>,
wallet_id: Option<String>,
memo: Option<String>,
}

pub struct Batch {
payments: Vec<Payment>,
client: GaloyClient,
/// price in btc/usd
price: Decimal,
}

impl Batch {
pub fn new(client: GaloyClient, price: Decimal) -> Self {
let payments: Vec<Payment> = vec![];
Self {
payments,
client,
price,
}
// Utility function to read and validate the CSV file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is redundant as it adds no additional information than the name of the function. Delete it - or alternatively if you want to publish this as a library turn in into a doc style comment (///) and add something useful.

pub fn validate_csv(
galoy_cli: &GaloyClient,
file: &str,
) -> anyhow::Result<(Vec<csv::StringRecord>, Wallet)> {
let mut reader = csv::ReaderBuilder::new().delimiter(b',').from_path(file)?;
let headers = reader.headers()?.clone();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why clone?


if &headers[0] != "username"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we could make this feature proof and go with lightning address? or maybe lightning address has not been implemented yet from the cli?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently haven't implemented Lightning Address in the CLI, but it's on our to-do list. Once we've got that (probably within this week), we'll tweak the batch payments to work with it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not be using magic values... all hard coded values must be declared as constants.

|| (&headers[1] != "cents" && &headers[1] != "sats")
|| (headers.len() == 3 && &headers[2] != "memo")
{
return Err(anyhow::anyhow!(
"CSV format not correct, requires: username, (cents or sats), memo(optional)"
));
}

pub fn add(&mut self, input: PaymentInput) {
self.payments.push(input.into());
}

pub fn add_csv(&mut self, filename: String) -> anyhow::Result<()> {
let file = File::open(filename)?;
let mut rdr = csv::Reader::from_reader(file);
for result in rdr.deserialize() {
let record: PaymentInput = result?;
self.add(record);
}

Ok(())
}

pub fn len(&self) -> usize {
self.payments.len()
}

pub fn is_empty(&self) -> bool {
self.payments.is_empty()
}
let wallet_type = if headers.get(1) == Some("cents") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we assuming that it we sent sats, it's from the btc wallet, and if we send cents, it's from the usd wallet?

I don't think we should make this assumption. we can send "100 dollar worth of bitcoin" from the bitcoin wallet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think typically people right not send "cents" or "cents equivalent", so we could start with this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Batch payments is built upon intraledger payments, which does not support sending cents from a Bitcoin wallet or sats from a USD wallet. Due to this, our current implementation assumes that if the amount is in cents, it is sent from the USD wallet and vice versa. Once we incorporate the capability for conversions in intraledger payments, we will extend the batch payments feature to allow this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be handled via FromStr / parse

Wallet::Usd
} else {
Wallet::Btc
};

pub fn populate_wallet_id(&mut self) -> anyhow::Result<()> {
for payment in self.payments.iter_mut() {
let username = payment.username.clone();
let query = &self.client.default_wallet(username);
match query {
Ok(value) => payment.wallet_id = Some(value.clone()),
Err(error) => bail!("error query {:?}", error),
}
}
let records: Vec<csv::StringRecord> = reader.records().collect::<Result<_, _>>()?;

Ok(())
}
// Validate each record
for record in &records {
let username = record
.get(0)
.ok_or(anyhow::anyhow!("Username is missing"))?;

pub fn populate_sats(&mut self) -> anyhow::Result<()> {
for payment in self.payments.iter_mut() {
let payment_btc: Decimal = payment.usd / self.price;
payment.sats = Some(payment_btc * dec!(100_000_000));
}
let amount = record.get(1).ok_or(anyhow::anyhow!("Amount is missing"))?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the syntax:

 record.get(1).context("Amount is missing")?;

more concise.
Need to use anyhow::Context for it to work.

amount
.parse::<Decimal>()
.map_err(|_| anyhow::anyhow!("Amount must be a number"))?;

Ok(())
// Check if the username exists
galoy_cli.default_wallet(username.to_string())?;
}

pub fn check_self_payment(&self) -> anyhow::Result<()> {
let me = self.client.me()?;

#[allow(deprecated)]
let me_username = match me.username {
Some(value) => value,
None => bail!("no username has been set"),
};

for payment in self.payments.iter() {
if me_username == payment.username {
println!("{:#?}", (me_username, &payment.username));
bail!("can't pay to self")
}
}
Ok((records, wallet_type))
}

Ok(())
pub fn check_sufficient_balance(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should also check the sending limits.

records: &[csv::StringRecord],
wallet_type: Wallet,
galoy_cli: &GaloyClient,
) -> anyhow::Result<()> {
let balance_info = galoy_cli.fetch_balance(Some(wallet_type), Vec::new())?;
let current_balance: Decimal = balance_info.iter().map(|info| info.balance).sum();

let mut total_payment_amount: Decimal = Decimal::new(0, 0);
for record in records {
let amount: Decimal = Decimal::from_str(record.get(1).unwrap_or_default())?;
total_payment_amount += amount;
}

pub fn check_limit(&self) -> anyhow::Result<()> {
todo!("Check limit. need API on the backend for it");
if total_payment_amount > current_balance {
return Err(anyhow::anyhow!("Insufficient balance in the wallet"));
}

pub fn check_balance(&self) -> anyhow::Result<()> {
let me = self.client.me()?;
let me_wallet_id = me.default_account.default_wallet_id;

let mut total_sats = dec!(0);
Ok(())
}

for payment in self.payments.iter() {
let sats = match payment.sats {
Some(value) => value,
None => bail!("sats needs to be populated first"),
};
total_sats += sats;
}
pub fn execute_batch_payment(
records: &[csv::StringRecord],
wallet_type: Wallet,
galoy_cli: &GaloyClient,
) -> anyhow::Result<()> {
for record in records {
let username = record
.get(0)
.ok_or(anyhow::anyhow!("Username is missing"))?;

let me_default_wallet = me
.default_account
.wallets
.iter()
.find(|wallet| wallet.id == me_wallet_id);

let balance_sats = match me_default_wallet {
Some(value) => value.balance,
None => bail!("no balance"),
};
if total_sats > balance_sats {
bail!(
"not enough balance, got {}, need {}",
balance_sats,
total_sats
)
}
let amount: Decimal = Decimal::from_str(record.get(1).unwrap_or_default())?;

Ok(())
}
let memo = record.get(2).map(|s| s.to_string());

pub fn show(&self) {
println!("{:#?}", &self.payments)
}

pub fn execute(&mut self) -> anyhow::Result<()> {
self.check_self_payment()?;
self.check_balance()?;

for Payment {
username,
memo,
usd,
sats,
..
} in self.payments.drain(..)
{
let amount = match sats {
Some(value) => value,
None => bail!("need sats amount"),
};
let res = &self
.client
.intraleger_send(username.clone(), amount, memo)
.context("issue sending intraledger")?;

println!(
"payment to {username} of sats {amount}, usd {usd}: {:?}",
res
);
match wallet_type {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are you dealing with partial failures? What if the first 5 out of 10 payments complete but then there is an error - how is this reported to avoid a double payment on re-try?

Wallet::Usd => {
galoy_cli.intraleger_usd_send(username.to_string(), amount, memo)?;
}
Wallet::Btc => {
galoy_cli.intraleger_send(username.to_string(), amount, memo)?;
}
}

Ok(())
}
Ok(())
}
84 changes: 61 additions & 23 deletions src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use reqwest::blocking::Client;

use log::info;
use rust_decimal::Decimal;
use std::net::TcpListener;
use std::{collections::HashSet, net::TcpListener};

pub mod queries;
pub use queries::*;
Expand All @@ -13,12 +13,24 @@ pub mod error;
pub use error::*;

pub mod batch;
pub use batch::Batch;
use crate::client::batch::*;

use self::query_me::WalletCurrency;
pub use self::query_me::WalletCurrency;

use crate::types::*;

pub mod server;

impl From<&WalletCurrency> for Wallet {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like putting From implementations into their own file eg src/client/convert.rs.

fn from(currency: &WalletCurrency) -> Self {
match currency {
WalletCurrency::USD => Wallet::Usd,
WalletCurrency::BTC => Wallet::Btc,
_ => panic!("Unsupported currency"),
}
}
}

pub struct GaloyClient {
graphql_client: Client,
api: String,
Expand Down Expand Up @@ -97,6 +109,46 @@ impl GaloyClient {
Ok(me)
}

pub fn fetch_balance(
&self,
wallet_type: Option<Wallet>,
wallet_ids: Vec<String>,
) -> anyhow::Result<Vec<WalletBalance>> {
let me = self.me()?;
let default_wallet_id = me.default_account.default_wallet_id;
let wallets = &me.default_account.wallets;

let wallet_ids_set: HashSet<_> = wallet_ids.into_iter().collect();

let balances: Vec<_> = wallets
.iter()
.filter(|wallet_info| {
wallet_ids_set.contains(&wallet_info.id)
|| wallet_type.as_ref().map_or(wallet_ids_set.is_empty(), |w| {
*w == Wallet::from(&wallet_info.wallet_currency)
})
})
.map(|wallet_info| WalletBalance {
currency: format!("{:?}", Wallet::from(&wallet_info.wallet_currency)),
balance: wallet_info.balance,
id: if wallet_info.wallet_currency == WalletCurrency::USD
|| wallet_info.wallet_currency == WalletCurrency::BTC
{
None
} else {
Some(wallet_info.id.clone())
},
default: wallet_info.id == default_wallet_id,
})
.collect();

if balances.is_empty() {
Err(anyhow::anyhow!("No matching wallet found"))
} else {
Ok(balances)
}
}

pub fn request_phone_code(&self, phone: String, nocaptcha: bool) -> std::io::Result<()> {
match nocaptcha {
false => {
Expand Down Expand Up @@ -286,26 +338,12 @@ impl GaloyClient {
Ok(())
}

// TODO: check if we can do self without &
pub fn batch(self, filename: String, price: Decimal) -> anyhow::Result<()> {
let mut batch = Batch::new(self, price);

batch.add_csv(filename).context("can't load file")?;

batch
.populate_wallet_id()
.context("cant get wallet id for all username")?;

batch
.populate_sats()
.context("cant set sats all payments")?;

println!("going to execute:");
batch.show();

batch.execute().context("can't make payment successfully")?;

Ok(())
pub fn batch_payment(self, file: String) -> anyhow::Result<String> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having batch as a submodule of client is IMO significantly bloating its responsibility. Typically a client is a thin wrapper around some network calls. More complex coordination that requires multiple round trips needs to be put in a use-case / app layer that uses the client as a dependency.

check_file_exists(&file)?;
let (reader, wallet_type) = validate_csv(&self, &file)?;
check_sufficient_balance(&reader, wallet_type.clone(), &self)?;
execute_batch_payment(&reader, wallet_type, &self)?;
Ok("Batch Payment Successful".to_string())
}

pub fn create_captcha_challenge(&self) -> Result<CaptchaChallenge, CliError> {
Expand Down
2 changes: 1 addition & 1 deletion src/client/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub use self::query_globals::QueryGlobalsGlobals;
query_path = "src/client/graphql/queries/me.graphql",
response_derives = "Debug, Serialize, PartialEq"
)]
pub(super) struct QueryMe;
pub struct QueryMe;
pub use self::query_me::QueryMeMe;

// mutations
Expand Down
Loading