rippled
rippled copied to clipboard
XRPLF
fixPayChanV1
This commit introduces a new fix amendment (fixPayChanV1) which prevents the creation of new PaymentChannelCreate transaction with a CancelAfter time less than the current ledger time. It piggy backs off of fix1571.
Once the amendment is activated, creating a new PaymentChannel will require that if you specify the CancelAfter time/value, that value must be greater than or equal to the current ledger time.
Currently users can create a payment channel where the CancelAfter time is before the current ledger time. This results in the payment channel being immediately closed on the next PaymentChannel transaction.
I did swap tecNO_PERMISSION for temBAD_EXPIRATION as I believe that is a more descriptive error message.
Although the issue can be resolved with an amendment, it's not necessarily a flaw at the protocol level. Instead, it's a potential user error. While many such errors can be detected, catching every single one might not be feasible. A strong case can be made for identifying these mistakes through checks in client libraries or at the API level. With this approach, it wouldn't be necessary to make changes that impact transaction processing. For instance, xrplcluster prevents transactions with extremely high fees, or payments to known scammers. Instead of trying to address every possible user error within the protocol, it's more practical to handle these in a layer in front of rippled. One of the benefits is that the fix can be rolled out immediately, permissionlessly and without total consensus.
Important notes:
- I don't see anything wrong with this amendment. It fixes a problem (even if it's a small problem with satisfactory alternative workarounds). I'm very happy to see developers open small fix PRs like this one.
- My comment offered a holistic view, considering both the benefits of the amendment and the practicality and immediacy of other solutions.
If @dangell7 (or anyone else in the community) would like this amendment, please proceed. Reopen this PR or open a new one. This protocol change can happen in parallel, and in addition to, any fixes at the client or API service layers.
@dangell7 - confirming - is this ready for review? (or are there any additional changes you're planning to make?)
@dangell7 - do you have an opinion on whether this should be considered for the next release (2.0.0) or if it's ok for it to wait for 2024?
(The first release in 2024 is expected for Jan or Feb.)
@intelliot I have no opinion. 2024 is fine. I don't think many use paychan but its nice that this is fixed at some point.
@dangell7
- This PR has conflicts that must be resolved.
- After resolving, please confirm that you still think this PR is ready to merge.