feat(namesys/IPNSPublisher): expose ability to set Sequence#962
feat(namesys/IPNSPublisher): expose ability to set Sequence#962
Conversation
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #962 +/- ##
==========================================
+ Coverage 61.34% 61.43% +0.08%
==========================================
Files 254 254
Lines 31709 31731 +22
==========================================
+ Hits 19452 19494 +42
+ Misses 10653 10636 -17
+ Partials 1604 1601 -3
... and 9 files with indirect coverage changes 🚀 New features to boost your workflow:
|
gsergey418
force-pushed
the
feat/10796-name-publish-sequence-option
branch
from
9450103 to
0d30983
Compare
|
@lidel Do you think it is necessary to include a note that setting a sequence number that is less than the current sequence of an existing record, may cause the new record not to be seen because it's sequence is lower? |
There was a problem hiding this comment.
@gammazero yes, that should be an error IMO
@gsergey418 afaik sequence is a monotonic clock, is there any case when it is something user wants to publish a record that will be ignored due to sequence not being the highest?
|
Triage notes:
|
There was a problem hiding this comment.
Thank you @gsergey418!
I added tests and security validation to prevent sequence numbers from going backwards.
Explainer: the original PR accepted any sequence number, creating unintentional vulnerability. If previous record exists, and we publish with lower sequence, the update will be ignored (at best) or we create unintentional replay attack (at worst).
Fixed this by requiring custom sequences to be greater than the current record's sequence, added proper error handling with ErrInvalidSequence, and wrote tests covering the validation scenarios to illustrate what is expected behavior.
Next steps
I think this is good to go but I would like to see tests in ipfs/kubo#10851 pass before we merge this.
lidel
changed the title
|
IPNS tests in CI in ipfs/kubo#10851 look good (only failures are due to lint/using forked dependency). I'm going to merge this and switch Kubo to commit from boxo main. |
4 participants
Fixes ipfs/kubo#10796. Added an option to PublishOptions(namesys) that allows for setting a custom sequence number for the IPNS record. Kubo PR