p2p/msgrate: return capacity as integer

[fjl]

Trying to fix the windows build failure.

[karalabe]

This seems the wrong approach. I think we just need to put a cap on the returned / consumed value.

[fjl]

But the capacity is really an integer. All uses of this method need to convert it to integer anyway, why not convert to integer at the source?

[karalabe]

I mean sure, you can convert the returned values to int, but that won't fix the CI issue :)

[fjl]

It does actually fix the CI issue, and it's because Capacity will no longer return negative values.

[fjl]

BTW, I'm talking about this issue: https://ci.appveyor.com/project/ethereum/go-ethereum/builds/39347915/job/ji6ellrc88eshfis#L1667

panic: runtime error: makeslice: cap out of range
goroutine 38971 [running]:
github.com/ethereum/go-ethereum/eth/downloader.(*queue).reserveHeaders(0x1618ea00, 0x1618eb40, 0x80000000, 0x161b7200, 0x19f33ba8, 0x161b7220, 0x0, 0x424b4301, 0x4)
	C:/projects/go-ethereum/eth/downloader/queue.go:494 +0xd4
github.com/ethereum/go-ethereum/eth/downloader.(*queue).ReserveBodies(0x1618ea00, 0x1618eb40, 0x80000000, 0x0, 0x17e30000)
	C:/projects/go-ethereum/eth/downloader/queue.go:458 +0xb1
github.com/ethereum/go-ethereum/eth/downloader.(*Downloader).fetchParts(0x17e39cc0, 0x161e8200, 0x17fcbf40, 0x161c33c0, 0x17fcbf38, 0x17fcbf68, 0x17fcbf60, 0x17fcbf58, 0x0, 0x101a06c, ...)
	C:/projects/go-ethereum/eth/downloader/downloader.go:1471 +0x1101
github.com/ethereum/go-ethereum/eth/downloader.(*Downloader).fetchBodies(0x17e39cc0, 0x1, 0x0, 0xe0e7e9, 0x20)
	C:/projects/go-ethereum/eth/downloader/downloader.go:1279 +0x2c6
github.com/ethereum/go-ethereum/eth/downloader.(*Downloader).syncWithPeer.func4(0xe711b0, 0x1)
	C:/projects/go-ethereum/eth/downloader/downloader.go:550 +0x6d
github.com/ethereum/go-ethereum/eth/downloader.(*Downloader).spawnSync.func1(0x17e39cc0, 0x161e8b40, 0x17ea2850)
	C:/projects/go-ethereum/eth/downloader/downloader.go:573 +0x67
created by github.com/ethereum/go-ethereum/eth/downloader.(*Downloader).spawnSync
	C:/projects/go-ethereum/eth/downloader/downloader.go:573 +0xc0

[karalabe]

Thought it returned something that overflown. Which part managed to go into negative?

[fjl]

I tried to reproduce this failure locally, but it doesn't repro for me unfortunately. It would be ideal to have a test in p2p/msgrate, but I don't really understand the code.

This is why I decided to fix it by adding math.Min(1, ...). This was also present in the original version in eth/downloader, but you removed it for some reason in p2p/msgrate.

[holiman]

func TestTracker(t *testing.T){
	tracker := NewTracker(nil, 1)
	tracker.Update(1, 1, 10000)
	cap := tracker.Capacity(1, 10000000)
	if int32(cap) < 0{
		t.Fatalf("Negative: %v", int32(cap))
	}
}

[karalabe]

Yeah, so it's essentially an int32 overflow. Guess we could add some upper limits somehow (e.g. maybe clamp to max int32, that should be enough for anything really and would solve the problem).

[fjl]

I tried this test, but it doesn't fail for me.

[fjl]

Updated to make the test fail without the previous fix, and added another fix to clamp the result to max positive int32 as well.

[fjl]

@karalabe PTAL

[karalabe]

LGTM!