Skip to content

Bitswap Complete (1.0.0 + 1.1.0) #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Bitswap Complete (1.0.0 + 1.1.0) #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const WantManager = require('./wantmanager')
const Network = require('./network')
const decision = require('./decision')

module.exports = class Bitwap {
class Bitswap {
constructor (p, libp2p, blockstore, peerBook) {
// the ID of the peer to act on behalf of
this.self = p
Expand All @@ -32,6 +32,7 @@ module.exports = class Bitwap {
// handle message sending
this.wm = new WantManager(this.network)

// stats
this.blocksRecvd = 0
this.dupBlocksRecvd = 0
this.dupDataRecvd = 0
Expand All @@ -42,7 +43,7 @@ module.exports = class Bitwap {

// handle messages received through the network
_receiveMessage (peerId, incoming, cb) {
cb = cb || (() => {})
cb = cb || noop
log('receiving message from %s', peerId.toB58String())
this.engine.messageReceived(peerId, incoming, (err) => {
if (err) {
Expand Down Expand Up @@ -100,15 +101,15 @@ module.exports = class Bitwap {
}

_updateReceiveCounters (block, cb) {
this.blocksRecvd ++
this.blocksRecvd++
this.blockstore.has(block.key, (err, has) => {
if (err) {
log('blockstore.has error: %s', err.message)
return cb(err)
}

if (has) {
this.dupBlocksRecvd ++
this.dupBlocksRecvd++
this.dupDataRecvd += block.data.length
return cb(new Error('Already have block'))
}
Expand Down Expand Up @@ -245,11 +246,16 @@ module.exports = class Bitwap {
return pull(
pull.asyncMap((block, cb) => {
this.blockstore.has(block.key, (err, exists) => {
if (err) return cb(err)
if (err) {
return cb(err)
}
cb(null, [block, exists])
})
}),
pull.filter((val) => !val[1]),
pull.filter((val) => {
const exists = val[1]
return !exists
}),
pull.map((val) => {
const block = val[0]

Expand Down Expand Up @@ -304,3 +310,7 @@ module.exports = class Bitwap {
this.engine.stop()
}
}

module.exports = Bitswap

function noop () {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't we agree that we would stop doing style refactorings that are based on personal preferences?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be wrong, but I don't see this as a style preference. Seeing cb = cb || noop is more simpler than cb = cb || (() => {}) everytime I want a noop.

I can revert back though.

As I said at the top of the PR: "Nothing to see here", which was meant to save time from CR, this is far from complete (note that I didn't add the "needs review" label).

5 changes: 3 additions & 2 deletions src/message/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ class BitswapMessage {
}
}

addBlock (block) {
this.blocks.set(mh.toB58String(block.key), block)
addBlock (block, hashAlg) {
const key = block.key(hashAlg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is going to be async with the new crypto. so might be better to base it on top of that

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be incredibly hard to work on top of a PR of PRs and do PR of PRs. One giant at a time, that is why we have git :)

this.blocks.set(mh.toB58String(key), block)
}

cancel (key) {
Expand Down
4 changes: 2 additions & 2 deletions test/browser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ function createRepo (id, done) {
})
})

const mainBlob = new Store(id)
const blocksBlob = new Store(`${id}/blocks`)
const mainBlob = new Store(id + Math.random())
const blocksBlob = new Store(`${id}/blocks` + Math.random())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was due to ipfs/js-ipfs#528 (comment)


dbs.push(id)

Expand Down
27 changes: 17 additions & 10 deletions test/index-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ module.exports = (repo) => {

before((done) => {
repo.create('hello', (err, r) => {
if (err) return done(err)
expect(err).to.not.exist
store = r.blockstore
done()
})
Expand All @@ -46,7 +46,7 @@ module.exports = (repo) => {
})

describe('receive message', () => {
it('simple block message', (done) => {
it.only('simple block message', (done) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: remove before merge

const me = PeerId.create({bits: 64})
const book = new PeerBook()
const bs = new Bitswap(me, libp2pMock, store, book)
Expand All @@ -56,23 +56,30 @@ module.exports = (repo) => {
const b1 = makeBlock()
const b2 = makeBlock()
const msg = new Message(false)
msg.addBlock(b1)
msg.addBlock(b2)
msg.addBlock(b1, 'sha2-256')
msg.addBlock(b2, 'sha2-256')

bs._receiveMessage(other, msg, (err) => {
if (err) throw err
expect(err).to.not.exist

expect(bs.blocksRecvd).to.be.eql(2)
expect(bs.dupBlocksRecvd).to.be.eql(0)

pull(
pull.values([b1, b1]),
pull.map((block) => store.getStream(block.key)),
pull.values([
b1,
b2
]),
pull.map((block) => {
return store.getStream(block.key('sha2-256'))
}),
pull.flatten(),
pull.collect((err, blocks) => {
if (err) return done(err)

expect(blocks).to.be.eql([b1, b1])
expect(err).to.not.exist
expect(blocks[0].key('sha2-256'))
.to.be.eql(b1.key('sha2-256'))
expect(blocks[1].key('sha2-256'))
.to.be.eql(b2.key('sha2-256'))
done()
})
)
Expand Down
6 changes: 3 additions & 3 deletions test/node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ const repo = {
}

require('./index-test')(repo)
require('./decision/engine-test')(repo)
require('./network/network.node.js')
require('./network/gen-bitswap-network.node.js')
// require('./decision/engine-test')(repo)
// require('./network/network.node.js')
// require('./network/gen-bitswap-network.node.js')