basic ID cmd #255
Merged
basic ID cmd #255
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@whyrusleeping we get the version information on handshake1. We should be able to request resending the information on handshake1 and handshake3, and implement |
|
(or, when we connect to a peer, we store it in the peer.Peer object. So, if (we're not connected to them, we try connecting) we have it around in the peer.Peer object we print it. |
|
Needs some locking, and travis failed. rebase, and otherwise LGTM!! |
| } else if p.privKey != nil { | ||
| return p.privKey.GetPublic() | ||
| } | ||
| return nil |
There was a problem hiding this comment.
nono let's not do this because we dont want to have to compute it every time. We could move to this:
if p.pubKey == nil && p.privKey != nil {
p.pubKey = p.privKey.GetPublic()
}
return p.pubKeybut did you actually fall into this case?
There was a problem hiding this comment.
I dont know if ive seen it occur, but it feels like the common sense thing to do
There was a problem hiding this comment.
sure. switch it to mine, so we store the new value
|
|
|
Panic on simply testing it. Protocol changes like this will mean talking to nodes like mars, which don't currently have this change. Clients should not panic. |
|
And: @maybebtc do you want new commands not to be added until #263 lands? |
|
Also, |
|
btw, this cmd is really great! it makes the system much more tangible |
|
@jbenet |
|
@maybebtc ok this cmd will have to wait then, as it currently panics. ( |
|
Should I migrate this over to the new commands system? Or wait until the PR lands? |
|
the PR is pretty big, but it wouldn't really hurt. |
|
@whyrusleeping ok this one is finally unblocked! 👍 |
whyrusleeping
force-pushed
the
id-cmd
branch
from
d200fa7
to
0de12b5
Compare
|
Alright, should be properly ported over to commands2 |
|
@whyrusleeping travis doesnt like you. Also, maybe add a sharness test? |
|
lol, travis failed because a bitswap test timed out, the stack trace shows over 500 goroutines spawned by that pubsub library we are using.. |
In that test there are 500 bitswap instances each waiting for 1 key. |
|
@whyrusleeping i merged this in and it doesn't work :/ need to specify arguments |
|
Fixed in: 21d2838 |
|
Ah, i guess i didnt really understand how that part of the commands worked. |
ariescodescream
pushed a commit
to ariescodescream/go-ipfs
that referenced
this pull request
Oct 23, 2021
fix tests on freebsd
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The ID command I implemented here doesnt have all the information that @jbenet mentioned he would like to have in #228, I wasnt sure how to access the version information without using the config (we wont be able to get configs from remote peers)