fix: ignore message false positive (#59)

* fix: ignore message false positive

This PR changes the `toString` encoding of message `seqno` to 'hex' so that a unique key is generated by `utils.msgId` for caching messages.

Converting the `seqno` to a utf8 string can sometimes result in the same string for different buffers. The following are two different seqno's that `js-ipfs` was sent from `go-ipfs` that demonstrates the problem:

```console
$ node
> const buf2 = Buffer.from('15603533e990dfe0', 'hex')
> const buf1 = Buffer.from('15603533e990dfde', 'hex')
> buf1.toString('utf8')
'\u0015`53���'
> buf2.toString('utf8')
'\u0015`53���'
> buf1.toString('utf8') === buf2.toString('utf8')
true
>
```

So sometimes we think we've seen the message when we haven't! 🤦‍♂️

License: MIT
Signed-off-by: Alan Shaw <[email protected]>

* fix: jsdoc for randomSeqno

License: MIT
Signed-off-by: Alan Shaw <[email protected]>

Author: Alan Shaw

src/index.js

@@ -9,7 +9,6 @@ const BaseProtocol = require('./base')
 const utils = require('./utils')
 const pb = require('./message')
 const config = require('./config')
-const Buffer = require('safe-buffer').Buffer
 
 const multicodec = config.multicodec
 const ensureArray = utils.ensureArray
@@ -88,7 +87,7 @@ class FloodSub extends BaseProtocol {
 
   _processRpcMessages (msgs) {
     msgs.forEach((msg) => {
-      const seqno = utils.msgId(msg.from, msg.seqno.toString())
+      const seqno = utils.msgId(msg.from, msg.seqno)
       // 1. check if I've seen the message, if yes, ignore
       if (this.cache.has(seqno)) {
         return
@@ -168,7 +167,7 @@ class FloodSub extends BaseProtocol {
       return {
         from: from,
         data: msg,
-        seqno: new Buffer(seqno),
+        seqno: seqno,
         topicIDs: topics
       }
     }

src/utils.js

@@ -8,23 +8,23 @@ exports = module.exports
 /**
  * Generatea random sequence number.
  *
- * @returns {string}
+ * @returns {Buffer}
  * @private
  */
 exports.randomSeqno = () => {
-  return crypto.randomBytes(20).toString('hex')
+  return crypto.randomBytes(20)
 }
 
 /**
  * Generate a message id, based on the `from` and `seqno`.
  *
  * @param {string} from
- * @param {string} seqno
+ * @param {Buffer} seqno
  * @returns {string}
  * @private
  */
 exports.msgId = (from, seqno) => {
-  return from + seqno
+  return from + seqno.toString('hex')
 }
 
 /**

test/utils.spec.js

@@ -10,13 +10,20 @@ describe('utils', () => {
     const first = utils.randomSeqno()
     const second = utils.randomSeqno()
 
-    expect(first).to.have.length(40)
-    expect(second).to.have.length(40)
+    expect(first).to.have.length(20)
+    expect(second).to.have.length(20)
     expect(first).to.not.eql(second)
   })
 
   it('msgId', () => {
-    expect(utils.msgId('hello', 'world')).to.be.eql('helloworld')
+    expect(utils.msgId('hello', Buffer.from('world'))).to.be.eql('hello776f726c64')
+  })
+
+  it('msgId should not generate same ID for two different buffers', () => {
+    const peerId = 'QmPNdSYk5Rfpo5euNqwtyizzmKXMNHdXeLjTQhcN4yfX22'
+    const msgId0 = utils.msgId(peerId, Buffer.from('15603533e990dfde', 'hex'))
+    const msgId1 = utils.msgId(peerId, Buffer.from('15603533e990dfe0', 'hex'))
+    expect(msgId0).to.not.eql(msgId1)
   })
 
   it('anyMatch', () => {