Fix depleted daily limit from unspent hits

[iWarpBTC]

If an invoice payment failed (for example an over-limit payment is attempted, or just there's no route) this is still recorded as a spent hit and it may deplete the daily limit. This PR fixes it.

Also now payment request of precisely maxWithdrawable amount doesn't pass since this is probably a PoS error or stealing, not a common payment.

[gorrdy]

Tried it and it's working as expected.

[gimme]

If an invoice payment failed (for example an over-limit payment is attempted, or just there's no route) this is still recorded as a spent hit and it may deplete the daily limit. This PR fixes it.

I don't see the code that fixes this.

Also now payment request of precisely maxWithdrawable amount doesn't pass since this is probably a PoS error or stealing, not a common payment.

Could you create a separate PR or issue for this? I think it needs some discussion.

[gorrdy]

You can see the code that fixes sit here. It is pretty straight forward. https://github.com/lnbits/boltcards/pull/4/files

[gimme]

I can only see the code for the unrelated issue, not what the title claims to fix.

[gorrdy]

Basically what this PR does is that it checks if the amount in invoice is lower than the card limit.

Currently it tries to do the payment but the payment obviously fails (if higher than the card limit). This would be ok but it also counts the amount as spent which is

1. not true
2. the payment counts to the daily limit - other payments may not go through because of the previous failed payment.

It doesn't make sense to try payment that i guaranteed to fail (above card tx limit). That's why there is the if statement at the beginning. Also it resolves the point 2. It won't mark the hit as spend.

if invoice.amount_msat < card.tx_limit * 1000:
         hit = await spend_hit(id=hit.id, amount=int(invoice.amount_msat / 1000))
         assert hit
         try:
             await pay_invoice(
                 wallet_id=card.wallet,
                 payment_request=pr,
                 max_sat=card.tx_limit,
                 extra={"tag": "boltcard", "hit": hit.id},
             )
             return {"status": "OK"}
         except Exception as exc:
             return {"status": "ERROR", "reason": f"Payment failed - {exc}"}
     else:
         return {"status": "ERROR", "reason": "Amount exceeds card limit"}

[gimme]

Oh, I see what you mean, but it doesn't fix if the payment fails for another reason than tx limit (e.g., like you mentioned, if there's no route).

[gorrdy]

This is true but it is not that big of a deal if the payment is reasonable amount. The problem is someone can disable your bolt card/bolt ring for a day just by a simple tap and requesting more sats than allowed.

It is not a big deal if there is 10k sats more of 300k daily limit spent... But this should also be resolved, that's right.

[iWarpBTC]

I believeI believe it would be quite safe to "unspent" a hit when an Exception occurs?

[gimme]

It should be pretty simple to only add the hit if the payment succeeds.

Otherwise, I guess it's fine to fix this first, but I still think we shouldn't prevent requests for precisely maxWithdrawable without more discussion about that.

[gimme]

I believeI believe it would be quite safe to "unspent" a hit when an Exception occurs?

Yes, I think so