Swarm network rewrite ethereum main repo relevant changes #654
Conversation
whisper/whisperv6/peer_test.go
Outdated
| @@ -101,7 +101,7 @@ var debugMode = false | |||
| var prevTime time.Time | |||
| var cntPrev int | |||
|
|
|||
| func TestSimulation(t *testing.T) { | |||
There was a problem hiding this comment.
@justelad we have to revert this on our end. Whisper team most probably has already fixed this.
whisper/whisperv5/peer_test.go
Outdated
| @@ -156,7 +156,7 @@ func initialize(t *testing.T) { | |||
|
|
|||
| err = node.server.Start() | |||
| if err != nil { | |||
| t.Fatalf("failed to start server %d.", i) | |||
There was a problem hiding this comment.
@justelad we have to revert this as well.
p2p/simulations/network.go
Outdated
| @@ -408,6 +404,18 @@ func (net *Network) GetNodes() (nodes []*Node) { | |||
| return nodes | |||
| } | |||
|
|
|||
| // GetNodes returns the existing nodes that are up | |||
| func (net *Network) GetUpNodes() (nodes []*Node) { | |||
There was a problem hiding this comment.
I asked about this method a few weeks ago, and it is still not used. I think we should not be committing dead code.
|
i think that travis will fail bluntly on the |
p2p/nat/natupnp_test.go
Outdated
| @@ -29,6 +29,7 @@ import ( | |||
| ) | |||
|
|
|||
| func TestUPNP_DDWRT(t *testing.T) { | |||
| t.Skip("broken") | |||
There was a problem hiding this comment.
This is passing on master, not sure why we are skipping it.
p2p/simulations/adapters/inproc.go
Outdated
| } | ||
|
|
||
| // tcpPipe creates an in process full duplex pipe based on a localhost TCP socket | ||
| func tcpPipe() (net.Conn, net.Conn, error) { |
There was a problem hiding this comment.
@nonsense go away as in go away with all of the code which depends on it? there's dependant code in pss_test and snapshot_sync_test
There was a problem hiding this comment.
We already have this in rlpx_test.go - we should export TCPPipe and reuse in this package.
| @@ -330,6 +330,7 @@ func doLint(cmdline []string) { | |||
| configs := []string{ | |||
| "--vendor", | |||
| "--tests", | |||
| "--deadline=10m", | |||
There was a problem hiding this comment.
@nonsense do we want to introduce this?
There was a problem hiding this comment.
Yes, I think there is no harm in it.
| l.h.Log(&Record{ | ||
| Time: time.Now(), | ||
| Lvl: lvl, | ||
| Msg: msg, | ||
| Ctx: newContext(l.ctx, ctx), | ||
| Call: stack.Caller(2), | ||
| Call: stack.Caller(skip), |
There was a problem hiding this comment.
@nonsense why was this change introduced? i mean, not better to have this adjustable from one spot instead of 12 others?
There was a problem hiding this comment.
This is needed, so that we have the Output method, which takes a different skip level.
There was a problem hiding this comment.
See function Respond in swarm/api/http/error.go
There was a problem hiding this comment.
ok, but maybe we could have a const for the default value of 2?
There was a problem hiding this comment.
@zelig it is explained above why we need it - See function Respond and the Output method. This helps us track the caller line more stacks up, than just 2.
|
@nonsense this is ready for another review |
internal/jsre/deps/web3.js
Outdated
| @@ -2307,7 +2307,7 @@ var toChecksumAddress = function (address) { | |||
| }; | |||
|
|
|||
| /** | |||
| * Transforms given string to valid 20 bytes-length addres with 0x prefix | |||
| * Transforms given string to valid 20 bytes-length address with 0x prefix | |||
There was a problem hiding this comment.
this change should be sumbitted to web3 js not here this file is updated from there.
Skip this one
| @@ -15,7 +15,7 @@ import ( | |||
|
|
|||
| const ( | |||
| timeFormat = "2006-01-02T15:04:05-0700" | |||
| termTimeFormat = "01-02|15:04:05" | |||
| termTimeFormat = "01-02|15:04:05.999999" | |||
There was a problem hiding this comment.
do we really need nanosecond resolution on log stamps?
There was a problem hiding this comment.
With all the race conditions we had to track and fix, this was very useful.
| l.h.Log(&Record{ | ||
| Time: time.Now(), | ||
| Lvl: lvl, | ||
| Msg: msg, | ||
| Ctx: newContext(l.ctx, ctx), | ||
| Call: stack.Caller(2), | ||
| Call: stack.Caller(skip), |
p2p/testing/protocolsession.go
Outdated
| @@ -91,7 +91,9 @@ func (s *ProtocolSession) trigger(trig Trigger) error { | |||
| errc := make(chan error) | |||
|
|
|||
| go func() { | |||
| log.Trace(fmt.Sprintf("trigger %v (%v)....", trig.Msg, trig.Code)) | |||
| @@ -61,7 +61,7 @@ const ( | |||
| // The approach taken here is to maintain a per-subscription linked list buffer | |||
| // shrinks on demand. If the buffer reaches the size below, the subscription is | |||
| // dropped. | |||
| maxClientSubscriptionBuffer = 8000 | |||
| maxClientSubscriptionBuffer = 20000 | |||
There was a problem hiding this comment.
please mention this is the PR synopsis as separate point
|
@zelig |
p2p/simulations/helpers/pipes.go
Outdated
| // You should have received a copy of the GNU Lesser General Public License | ||
| // along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| package helpers |
There was a problem hiding this comment.
this is a bad name for the package. maybe just call it pipes?
|
@justelad thanks for taking care of this. Just two remarks from my side:
The rest I am happy with and hopefully will get approved on ethereum/go-ethereum side. |
…all non-swarm package changes relevant for the main go-ethereum repository for the upcoming swarm-network-rewrite merge into master rpc/client: increased maxClientSubscriptionBuffer to 20000
Fix a spelling mistake in comment
…f bool (#16757) Fixes #16756
This commit adds many comments and removes unused code. It also removes the EmptyHash function, which had some uses but was silly.
This removes a golint warning: type name will be used as trie.TrieSync by other packages, and that stutters; consider calling this Sync. In hexToKeybytes len(hex) is even and (even+1)/2 == even/2, remove the +1.
acud
force-pushed
the
swarm-network-rewrite-eth-relevant-changes
branch
from
e50646c to
0c7f221
Compare
No description provided.