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

Issue #470 Add validation on /settleorder #567

Merged
merged 2 commits into from
Aug 16, 2024

Conversation

AndreaDiazCorreia
Copy link
Contributor

Implement check in settleorder command to disallow settlement of orders with PAID_HOLD_INVOICE status

disallow settlement of orders with PAID_HOLD_INVOICE status
Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

Hi @AndreaDiazCorreia thank you for your contribution, I can give you some context about this issue.

/settleorder is an command only available for admins, what this command does is changes the status of the order to PAID_HOLD_INVOICE and settles the the hold invoice paid by the seller, after this automatically the bot tries to pay buyer invoice, we don't want that an admin by mistake execute it twice and the bot try to pay the invoice again (technical not possible but in the future we want to implement offers)

so only admin can settle, the seller don't need to get a message if the invoice is on PAID_HOLD_INVOICE because the seller already received the message the first time the order changed to that status.

We should send a message to the command executor ctx.admin, for this you need to add new locales on all languages and a ctx.reply() would do the job 😃

@AndreaDiazCorreia
Copy link
Contributor Author

Hi @grunch I made the changes you mentioned

Copy link
Member

@Catrya Catrya left a comment

Choose a reason for hiding this comment

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

Hi, I think we need to modify the message that the bot sends, because in paid-hold-invoice status the invoice has not been paid yet.
May be its ok translated in all languages: This order has already been settled

Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

LGTM!

@grunch
Copy link
Member

grunch commented Aug 16, 2024

@AndreaDiazCorreia please add a 50000 sats lightning invoice in a new comment of this PR

@AndreaDiazCorreia
Copy link
Contributor Author

lnbc500u1pnt7lzapp549mtely342scjpnvz57h9vpzqkahzc9j594muawk7h66jeq3n6nsdqqcqzzsxqyz5vqsp5nahu0ktfeszaqaqlzyv8kt42hpsnjw7ya0yfdmv9v08zlsqjfwnq9qyyssqx3qjhjq90s7lxpsple724rehk8dlxmpzzwam7f4prkke2guaeg9n6cmacp3t3pa4s98hsry54a6j5r3xjjqmnp9wljplxxm6jsx8gsqqeweuez

@AndreaDiazCorreia
Copy link
Contributor Author

or if you have a lightning address: [email protected]

@grunch
Copy link
Member

grunch commented Aug 16, 2024

lnbc500u1pnt7lzapp549mtely342scjpnvz57h9vpzqkahzc9j594muawk7h66jeq3n6nsdqqcqzzsxqyz5vqsp5nahu0ktfeszaqaqlzyv8kt42hpsnjw7ya0yfdmv9v08zlsqjfwnq9qyyssqx3qjhjq90s7lxpsple724rehk8dlxmpzzwam7f4prkke2guaeg9n6cmacp3t3pa4s98hsry54a6j5r3xjjqmnp9wljplxxm6jsx8gsqqeweuez

done! thank you 😃

preimage: ed01bd59a14fb7f49080f415fe8335e84cd3c6310ca6560606cc8d3c93ce7924

@grunch grunch merged commit 5fa48c1 into lnp2pBot:main Aug 16, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants