Skip to content
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.

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

multiaddrs with .../ipfs/... #608

Merged
merged 13 commits into from
Feb 1, 2015
Merged

multiaddrs with .../ipfs/... #608

merged 13 commits into from
Feb 1, 2015

Conversation

jbenet
Copy link
Member

@jbenet jbenet commented Jan 21, 2015

This commit adds multiaddr support for "proper ipfs addresses", and
tries it out in bootstrap peers.

⚠️ this commit makes your current configs unusable, as the default
bootstrap peers. You may need to edit your config.

Go from:

Bootstrap: [
  {
    "Address": "/ip4/104.131.131.82/tcp/4001",
    "PeerID": "QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ"
  }
]

To:

Bootstrap: [
  "/ip4/104.131.131.82/tcp/4001/ipfs/QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ"
]

Note the .../ipfs/...

@jbenet jbenet added the status/in-progress In progress label Jan 21, 2015
@jbenet
Copy link
Member Author

jbenet commented Jan 21, 2015

This builds on #583

@jbenet
Copy link
Member Author

jbenet commented Jan 21, 2015

The relevant part is only fd041ec...d9b0c54 -- will be clearer once #583 merges.

@whyrusleeping
Copy link
Member

Interesting that the network test failed... not entirely sure why that would happen. "files didnt match"

@jbenet
Copy link
Member Author

jbenet commented Jan 21, 2015

i think it's the reuse port-- sec

@jbenet
Copy link
Member Author

jbenet commented Jan 21, 2015

ah no-- it was a test case fix. we'll see I guess

@jbenet
Copy link
Member Author

jbenet commented Jan 21, 2015

Error: failed to parse ip4:  failed to parse ip4 addr: 

It may be some config / bootstrap addr problem.

@btc
Copy link
Contributor

btc commented Jan 21, 2015

The network test has manually-defined bootstrap configs.

@jbenet jbenet force-pushed the ipfs-multiaddr branch 3 times, most recently from 724b403 to 99c991c Compare January 23, 2015 13:45
@jbenet
Copy link
Member Author

jbenet commented Jan 23, 2015

RFCR here @whyrusleeping or @briantigerchow

And, @briantigerchow not sure what's going wrong with the network test. this error is different than before. any ideas? looks like it broke fig:

Exception in thread Thread-1 (most likely raised during interpreter shutdown):
Traceback (most recent call last):
  File "/code/build/fig/out00-PYZ.pyz/threading", line 552, in __bootstrap_inner
  File "/code/build/fig/out00-PYZ.pyz/threading", line 505, in run
  File "/code/build/fig/out00-PYZ.pyz/fig.cli.multiplexer", line 41, in _enqueue_output
  File "/code/build/fig/out00-PYZ.pyz/fig.cli.log_printer", line 59, in _make_log_generator
  File "/code/build/fig/out00-PYZ.pyz/fig.container", line 141, in wait
  File "/code/build/fig/out00-PYZ.pyz/docker.client", line 940, in wait
  File "/code/build/fig/out00-PYZ.pyz/docker.client", line 73, in _post
  File "/code/build/fig/out00-PYZ.pyz/requests.sessions", line 425, in post
  File "/code/build/fig/out00-PYZ.pyz/requests.sessions", line 383, in request
  File "/code/build/fig/out00-PYZ.pyz/requests.sessions", line 486, in send
  File "/code/build/fig/out00-PYZ.pyz/requests.adapters", line 374, in send
<type 'exceptions.AttributeError'>: 'NoneType' object has no attribute 'error'

@mappum
Copy link
Contributor

mappum commented Jan 23, 2015

Looks like these changes break ipfs bootstrap. I'm not sure if it affects ipfs swarm since it's not connecting me to anyone.

➜  ipfs git:(ipfs-multiaddr) ✗ ipfs bootstrap add /ip4/104.131.131.82/tcp/4001/ipfs/QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ
Error: json: cannot unmarshal object into Go value of type config.BootstrapPeer
➜  ipfs git:(ipfs-multiaddr) ✗ ipfs bootstrap list
Error: json: cannot unmarshal object into Go value of type config.BootstrapPeer

@btc
Copy link
Contributor

btc commented Jan 23, 2015

@jbenet

re: network test error.

On first glance, that looks like an actual bug/error.

@jbenet
Copy link
Member Author

jbenet commented Jan 24, 2015

@mappum

Looks like these changes break ipfs bootstrap. I'm not sure if it affects ipfs swarm since it's not connecting me to anyone.

It changes the config format. can you try removing the bootstrap entry from config and retrying? Actually, can you test whether:

ipfs bootstrap rm --all

works? maybe try an

ipfs config 

version. (need to find out exactly what we can tell users)

@mappum
Copy link
Contributor

mappum commented Jan 24, 2015

The error happens for all commands if there is a config file with the old bootstrap format. I deleted my ~/.go-ipfs directory, and ipfs init'd with this branch, and the error still happens on ipfs bootstrap (but only if the daemon is running).

@jbenet jbenet modified the milestone: α Jan 24, 2015
@jbenet jbenet self-assigned this Jan 24, 2015
@whyrusleeping
Copy link
Member

I may have to update some parsing in ipfs ping due to this change.

@jbenet
Copy link
Member Author

jbenet commented Jan 31, 2015

I've rebased this. I want to get it merged in today.

  • @mappum i couldnt get your error. works fine for me? (did you reboot your daemon earlier on?)
  • @whyrusleeping what code? want to push it onto this branch?

@jbenet
Copy link
Member Author

jbenet commented Feb 1, 2015

fixed a bug:

index 7ae4fde..b1d1f25 100644
--- a/core/bootstrap.go
+++ b/core/bootstrap.go
@@ -233,7 +233,7 @@ func toPeerInfo(bp config.BootstrapPeer) peer.PeerInfo {
    // of the codebase currently uses addresses without the peerid part.
    m := bp.Multiaddr()
    s := ma.Split(m)
-   m = ma.Join(s[len(s)-1])
+   m = ma.Join(s[:len(s)-1]...)

    return peer.PeerInfo{
        ID:    bp.ID(),

@whyrusleeping
Copy link
Member

Im getting the same problems as @mappum cant bootstrap to anyone

@whyrusleeping
Copy link
Member

Pushed my fix for pings address parsing here

⚠️ this commit makes your current configs unusable, as the
default bootstrap peers. You may need to edit your config.

Go from:

```js
Bootstrap: [
  {
    "Address": "/ip4/104.131.131.82/tcp/4001",
    "PeerID": "QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ"
  }
]
```

To:
```js
Bootstrap: [
  "/ip4/104.131.131.82/tcp/4001/ipfs/QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ"
]
```
@jbenet
Copy link
Member Author

jbenet commented Feb 1, 2015

@whyrusleeping @mappum saw what you meant. fixed in 188d336

jbenet added a commit that referenced this pull request Feb 1, 2015
@jbenet jbenet merged commit fbf1a50 into master Feb 1, 2015
@jbenet jbenet removed the status/in-progress In progress label Feb 1, 2015
@jbenet jbenet deleted the ipfs-multiaddr branch February 1, 2015 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants