Skip to content

Commit c8cee6a

Browse files
dirkmcdirkmc
committed
fix: use consistent encoding for cid comparison
BREAKING CHANGE: Emitted events have different bytes The emitted events contain the stringified version of the CID, as we change it to the base encoding the CID has, those bytes may be different to previous versions of this module. Though this shouldn't have any impact on any other modules as the events are only used internally.
1 parent 1e6d2ae commit c8cee6a

File tree

9 files changed

+95
-24
lines changed

9 files changed

+95
-24
lines changed

‎src/notifications.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ class Notifications extends EventEmitter {
3434
* @return {void}
3535
*/
3636
hasBlock (block) {
37-
const str = `block:${block.cid}`
37+
const cidStr = block.cid.toString('base58btc')
38+
const str = `block:${cidStr}`
3839
this._log(str)
3940
this.emit(str, block)
4041
}
@@ -49,7 +50,7 @@ class Notifications extends EventEmitter {
4950
* @returns {void}
5051
*/
5152
wantBlock (cid, onBlock, onUnwant) {
52-
const cidStr = cid.toString()
53+
const cidStr = cid.toString('base58btc')
5354
this._log(`wantBlock:${cidStr}`)
5455

5556
this._unwantListeners[cidStr] = () => {
@@ -80,7 +81,7 @@ class Notifications extends EventEmitter {
8081
* @returns {void}
8182
*/
8283
unwantBlock (cid) {
83-
const str = `unwant:${cid}`
84+
const str = `unwant:${cid.toString('base58btc')}`
8485
this._log(str)
8586
this.emit(str)
8687
}

‎src/types/message/entry.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ module.exports = class BitswapMessageEntry {
2828
}
2929

3030
get [Symbol.toStringTag] () {
31-
const cidStr = this.cid.toBaseEncodedString()
32-
31+
const cidStr = this.cid.toString('base58btc')
3332
return `BitswapMessageEntry ${cidStr} <cancel: ${this.cancel}, priority: ${this.priority}>`
3433
}
3534

‎src/types/message/index.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class BitswapMessage {
2828

2929
addEntry (cid, priority, cancel) {
3030
assert(cid && CID.isCID(cid), 'must be a valid cid')
31-
const cidStr = cid.buffer.toString()
31+
const cidStr = cid.toString('base58btc')
3232

3333
const entry = this.wantlist.get(cidStr)
3434

@@ -42,13 +42,13 @@ class BitswapMessage {
4242

4343
addBlock (block) {
4444
assert(Block.isBlock(block), 'must be a valid cid')
45-
const cidStr = block.cid.buffer.toString()
45+
const cidStr = block.cid.toString('base58btc')
4646
this.blocks.set(cidStr, block)
4747
}
4848

4949
cancel (cid) {
5050
assert(CID.isCID(cid), 'must be a valid cid')
51-
const cidStr = cid.buffer.toString()
51+
const cidStr = cid.toString('base58btc')
5252
this.wantlist.delete(cidStr)
5353
this.addEntry(cid, 0, true)
5454
}

‎src/types/wantlist/entry.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class WantListEntry {
2828

2929
// So that console.log prints a nice description of this object
3030
get [Symbol.toStringTag] () {
31-
const cidStr = this.cid.toBaseEncodedString()
31+
const cidStr = this.cid.toString('base58btc')
3232
return `WantlistEntry <key: ${cidStr}, priority: ${this.priority}, refs: ${this._refCounter}>`
3333
}
3434

‎src/types/wantlist/index.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ class Wantlist {
1414
}
1515

1616
add (cid, priority) {
17-
const cidStr = cid.buffer.toString()
17+
const cidStr = cid.toString('base58btc')
1818
const entry = this.set.get(cidStr)
1919

2020
if (entry) {
@@ -29,7 +29,7 @@ class Wantlist {
2929
}
3030

3131
remove (cid) {
32-
const cidStr = cid.buffer.toString()
32+
const cidStr = cid.toString('base58btc')
3333
const entry = this.set.get(cidStr)
3434

3535
if (!entry) {
@@ -68,7 +68,7 @@ class Wantlist {
6868
}
6969

7070
contains (cid) {
71-
const cidStr = cid.buffer.toString()
71+
const cidStr = cid.toString('base58btc')
7272
return this.set.get(cidStr)
7373
}
7474
}

‎test/bitswap-mock-internals.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ describe('bitswap with mocks', function () {
109109

110110
const wl = bs.wantlistForPeer(other)
111111

112-
expect(wl.has(b1.cid.buffer.toString())).to.eql(true)
113-
expect(wl.has(b2.cid.buffer.toString())).to.eql(true)
112+
expect(wl.has(b1.cid.toString('base58btc'))).to.eql(true)
113+
expect(wl.has(b2.cid.toString('base58btc'))).to.eql(true)
114114

115115
done()
116116
})

‎test/notifications.spec.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ const expect = chai.expect
77
const map = require('async/map')
88
const parallel = require('async/parallel')
99
const PeerId = require('peer-id')
10+
const CID = require('cids')
11+
const Block = require('ipfs-block')
1012

1113
const Notifications = require('../src/notifications')
1214

@@ -71,4 +73,39 @@ describe('Notifications', () => {
7173
n.unwantBlock(b.cid)
7274
})
7375
})
76+
77+
describe('wantBlock with same cid derived from distinct encodings', () => {
78+
it('receive block', (done) => {
79+
const n = new Notifications(peerId)
80+
const cid = new CID(blocks[0].cid.toV1().toString('base64'))
81+
const b = new Block(blocks[0].data, cid)
82+
83+
const cid2 = new CID(b.cid.toString('base32'))
84+
n.wantBlock(cid2, (block) => {
85+
expect(b).to.eql(block)
86+
87+
// check that internal cleanup works as expected
88+
expect(Object.keys(n._blockListeners)).to.have.length(0)
89+
expect(Object.keys(n._unwantListeners)).to.have.length(0)
90+
done()
91+
}, () => {
92+
done(new Error('should never happen'))
93+
})
94+
95+
n.hasBlock(b)
96+
})
97+
98+
it('unwant block', (done) => {
99+
const n = new Notifications()
100+
const cid = new CID(blocks[0].cid.toV1().toString('base64'))
101+
const b = new Block(blocks[0].data, cid)
102+
103+
const cid2 = new CID(b.cid.toString('base32'))
104+
n.wantBlock(cid2, () => {
105+
done(new Error('should never happen'))
106+
}, done)
107+
108+
n.unwantBlock(b.cid)
109+
})
110+
})
74111
})

‎test/types/message.spec.js

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,15 +92,15 @@ describe('BitswapMessage', () => {
9292
expect(msg.full).to.equal(true)
9393
expect(Array.from(msg.wantlist))
9494
.to.eql([[
95-
cid0.buffer.toString(),
95+
cid0.toString('base58btc'),
9696
new BitswapMessage.Entry(cid0, 0, false)
9797
]])
9898

9999
expect(
100100
Array.from(msg.blocks).map((b) => [b[0], b[1].data])
101101
).to.eql([
102-
[cid1.buffer.toString(), b1.data],
103-
[cid2.buffer.toString(), b2.data]
102+
[cid1.toString('base58btc'), b1.data],
103+
[cid2.toString('base58btc'), b2.data]
104104
])
105105

106106
done()
@@ -137,15 +137,15 @@ describe('BitswapMessage', () => {
137137
expect(msg.full).to.equal(true)
138138
expect(Array.from(msg.wantlist))
139139
.to.eql([[
140-
cid0.buffer.toString(),
140+
cid0.toString('base58btc'),
141141
new BitswapMessage.Entry(cid0, 0, false)
142142
]])
143143

144144
expect(
145145
Array.from(msg.blocks).map((b) => [b[0], b[1].data])
146146
).to.eql([
147-
[cid1.buffer.toString(), b1.data],
148-
[cid2.buffer.toString(), b2.data]
147+
[cid1.toString('base58btc'), b1.data],
148+
[cid2.toString('base58btc'), b2.data]
149149
])
150150

151151
done()
@@ -209,6 +209,23 @@ describe('BitswapMessage', () => {
209209
expect(m1.equals(m2)).to.equal(false)
210210
done()
211211
})
212+
213+
it('true, same cid derived from distinct encoding', (done) => {
214+
const b = blocks[0]
215+
const cid = cids[0].toV1()
216+
const cid1 = new CID(cid.toBaseEncodedString('base32'))
217+
const cid2 = new CID(cid.toBaseEncodedString('base64'))
218+
const m1 = new BitswapMessage(true)
219+
const m2 = new BitswapMessage(true)
220+
221+
m1.addEntry(cid1, 1)
222+
m2.addEntry(cid2, 1)
223+
224+
m1.addBlock(b)
225+
m2.addBlock(b)
226+
expect(m1.equals(m2)).to.equal(true)
227+
done()
228+
})
212229
})
213230

214231
describe('BitswapMessageEntry', () => {

‎test/types/wantlist.spec.js

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ describe('Wantlist', () => {
8686
expect(
8787
Array.from(wm.entries())
8888
).to.be.eql([[
89-
b.cid.buffer.toString(),
89+
b.cid.toString('base58btc'),
9090
new Wantlist.Entry(b.cid, 2)
9191
]])
9292
})
@@ -101,8 +101,8 @@ describe('Wantlist', () => {
101101
expect(
102102
Array.from(wm.sortedEntries())
103103
).to.be.eql([
104-
[b1.cid.buffer.toString(), new Wantlist.Entry(b1.cid, 1)],
105-
[b2.cid.buffer.toString(), new Wantlist.Entry(b2.cid, 1)]
104+
[b1.cid.toString('base58btc'), new Wantlist.Entry(b1.cid, 1)],
105+
[b2.cid.toString('base58btc'), new Wantlist.Entry(b2.cid, 1)]
106106
])
107107
})
108108

@@ -126,10 +126,27 @@ describe('Wantlist', () => {
126126
expect(
127127
Array.from(wm.entries())
128128
).to.be.eql([[
129-
cid.buffer.toString(),
129+
cid.toString('base58btc'),
130130
new Wantlist.Entry(cid, 2)
131131
]])
132132
done()
133133
})
134134
})
135+
136+
it('matches same cid derived from distinct encodings', () => {
137+
// Base 64
138+
const id1 = 'mAVUSIKlIkE8vD0ebj4GXaUswGEsNLtHBzSoewPuF0pmhkqRH'
139+
// Base 32
140+
const id2 = 'bafkreifjjcie6lypi6ny7amxnfftagclbuxndqonfipmb64f2km2devei4'
141+
142+
const cid1 = new CID(id1)
143+
const cid2 = new CID(id2)
144+
wm.add(cid1, 2)
145+
expect(wm.contains(cid1)).to.exist()
146+
expect(wm.contains(cid2)).to.exist()
147+
148+
wm.remove(cid1)
149+
expect(wm.contains(cid1)).not.to.exist()
150+
expect(wm.contains(cid2)).not.to.exist()
151+
})
135152
})

0 commit comments

Comments
 (0)