-
Notifications
You must be signed in to change notification settings - Fork 24
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
feat: add bria-client to stablesats #411
Conversation
pub async fn onchain_address(&mut self) -> Result<OnchainAddress, BriaClientError> { | ||
match self.get_address().await { | ||
Ok(addr) => Ok(addr), | ||
Err(_) => self.new_address().await, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't eat this error - only do new_address if the error is NotFound.
bria-client/src/client/config.rs
Outdated
#[serde(default = "default_url")] | ||
pub url: String, | ||
#[serde(default)] | ||
pub key: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we be more specific here? profile_api_key
perhaps?
bria-client/src/client/config.rs
Outdated
#[serde(default)] | ||
pub payout_queue_name: String, | ||
#[serde(default)] | ||
pub external_id: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about onchain_address_external_id
?
.await | ||
.map_err(|_| BriaClientError::ConnectionError(config.url.clone()))?; | ||
|
||
if config.key.is_empty() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is too late to validate. It should be asserted in cli.
bria-client/src/error.rs
Outdated
#[error("Couldn't connect to bria at url: {0}")] | ||
ConnectionError(String), | ||
#[error("Bria key cannot be empty")] | ||
EmptyKey, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should need this error.
bria-client/src/error.rs
Outdated
#[error("Couldn't create MetadataValue")] | ||
CouldNotCreateMetadataValue, | ||
#[error("Couldn't find address for the given external_id")] | ||
AddressNotFound, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When is this returned? This should be handled internally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when we call onchain_address()
, it first calls get_address()
. suppose we don't have an address for a given onchain_address_external_id
we use Err(BriaClientError::AddressNotFound) => self.new_address().await
to call new_address()
bria-client/tests/bria_client.rs
Outdated
|
||
fn client_configuration() -> BriaClientConfig { | ||
let url = "http://localhost:2742".to_string(); | ||
let key = env::var("BRIA_KEY").expect("BRIA_KEY not set"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be the dev key for tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cli/src/config.rs
Outdated
@@ -62,7 +67,7 @@ impl Config { | |||
}; | |||
|
|||
config.db.pg_con = stablesats_pg_con; | |||
|
|||
config.bria.key = bria_key; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is where to validate that the key exists.
let memo: String = format!("deposit of {amount_in_sats} sats to OKX"); | ||
let _ = galoy | ||
.send_onchain_payment(deposit_address, amount_in_sats, Some(memo), 1) | ||
let amount_in_sats = (amount * SATS_PER_BTC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conversion should happen in the client. Accept a decimal in the interface.
.abs() | ||
.to_u64() | ||
.expect("should always convert to u64"); | ||
let metadata: serde_json::Value = json!({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This metadata should be generated in the client.
aa9f2e0
to
3596cab
Compare
3596cab
to
2376532
Compare
2376532
to
3e436a3
Compare
Adds bria to stablesats