add support for RGB zero asset amount invoices#101
add support for RGB zero asset amount invoices#101bitwalt wants to merge 1 commit intoRGB-Tools:masterfrom
Conversation
src/routes.rs
Outdated
| return Err(APIError::InvalidAmount(format!( | ||
| "amt_msat cannot be less than {INVOICE_MIN_MSAT} when transferring an RGB asset" | ||
| ))); | ||
| } |
There was a problem hiding this comment.
the previous code looks fine to me, it's not preventing the creation of an invoice with no asset amount, and the new check is preventing it from being set while reporting an amt_msat error
please revert this
| pub(crate) struct SendPaymentRequest { | ||
| pub(crate) invoice: String, | ||
| pub(crate) amt_msat: Option<u64>, | ||
| pub(crate) asset_id: Option<String>, |
There was a problem hiding this comment.
since this also enables paying RGB assets to invoices specifying neither the asset_id nor the asset_amount, would you please cover this case as well in a test?
please also update the openapi spec (where amt_msat is missing already)
There was a problem hiding this comment.
I see the updated openapi spec but not the test case which uses an invoice with asset_id and asset_amount both set to None, was that intentional?
src/routes.rs
Outdated
| (Some(rgb_contract_id), None) => { | ||
| if amt_msat < INVOICE_MIN_MSAT { | ||
| return Err(APIError::InvalidAmount(format!( | ||
| "msat amount in invoice sending an RGB asset cannot be less than {INVOICE_MIN_MSAT}" |
There was a problem hiding this comment.
| "msat amount in invoice sending an RGB asset cannot be less than {INVOICE_MIN_MSAT}" | |
| "amt_msat in invoice sending an RGB asset cannot be less than {INVOICE_MIN_MSAT}" |
src/routes.rs
Outdated
| .map_err(|_| APIError::InvalidAssetID(asset_id.clone()))?; | ||
| if payload_contract_id != rgb_contract_id { | ||
| return Err(APIError::InvalidInvoice(s!( | ||
| "invoice RGB contract ID doesn't match request asset_id" |
There was a problem hiding this comment.
| "invoice RGB contract ID doesn't match request asset_id" | |
| "invoice RGB contract ID doesn't match the requested one" |
src/test/invoice.rs
Outdated
| #[serial_test::serial] | ||
| #[tokio::test(flavor = "multi_thread", worker_threads = 1)] | ||
| #[traced_test] | ||
| async fn rgb_invoice_without_asset_amount_can_be_paid_with_sendpayment_asset_amount() { |
There was a problem hiding this comment.
please try to include this in an existing test, to avoid paying the cost of setting up a new one
nicbus
changed the title
bitwalt
force-pushed
the
rgb-zero-amount-sendpayment-fix
branch
from
1866139 to
3cf7a3d
Compare
|
Thanks for the review, suggested changes has been addressed |
| pub(crate) struct SendPaymentRequest { | ||
| pub(crate) invoice: String, | ||
| pub(crate) amt_msat: Option<u64>, | ||
| pub(crate) asset_id: Option<String>, |
There was a problem hiding this comment.
I see the updated openapi spec but not the test case which uses an invoice with asset_id and asset_amount both set to None, was that intentional?
bitwalt
force-pushed
the
rgb-zero-amount-sendpayment-fix
branch
from
3cf7a3d to
571728f
Compare
|
Ok requested changes should be addressed now, let me know if its okay or some more cases are needed in the test. I included the case with asset_id and asset_amount both set to None that was missing |
2 participants
This PR reproduce #100 and provide a fix for it