Describe the bug
There is a clause in the BIP-326 spec that has been excluded in our AFS implementation:
# Initialize all nsequence to indicate the requested RBF state
# nsequence can not be 2**32 - 1 in order for nlocktime to take effect
for input in transaction.inputs:
if rbf_set:
input.nsequence = 2**32 - 3
else:
input.nsequence = 2**32 - 2
This is bad.
When all input nSequence values are Sequence::MAX, the nLockTime value has no effect (still consensus-valid, but nLockTime is not enforced), essentially making the locktime-branch of AFS a no-op.
Expected behavior
AFS should never complete the locktime-branch with all input sequence values as Sequence::MAX.
Proposed Solution
The claude in BIP-326 is aggressive with setting nSequence values. For us, I think it's better to retain as much user intent as possible. I.e. what if the input needs to satisfy CSV?
I propose only changing nSequence if it is set to Sequence::MAX, and only doing so in the AFS locktime-branch.
I think it's better to have #66 and #65 merged before tackling this issue.
Describe the bug
There is a clause in the BIP-326 spec that has been excluded in our AFS implementation:
This is bad.
When all input
nSequencevalues areSequence::MAX, thenLockTimevalue has no effect (still consensus-valid, butnLockTimeis not enforced), essentially making the locktime-branch of AFS a no-op.Expected behavior
AFS should never complete the locktime-branch with all input sequence values as
Sequence::MAX.Proposed Solution
The claude in BIP-326 is aggressive with setting nSequence values. For us, I think it's better to retain as much user intent as possible. I.e. what if the input needs to satisfy CSV?
I propose only changing
nSequenceif it is set toSequence::MAX, and only doing so in the AFS locktime-branch.I think it's better to have #66 and #65 merged before tackling this issue.