Commit Graph

58 Commits

Author SHA1 Message Date
Ethan Buchman e2d7f1aa41 cmn: fix race 2017-12-21 14:21:15 -05:00
Ethan Buchman b0b740210c cmn: fix repeate timer test with manual ticker 2017-12-21 11:15:17 -05:00
Ethan Buchman a25ed5ba1b cmn: fix race condition in prng 2017-12-21 10:02:25 -05:00
Anton Kaliaev e17e8e425f
Revert "Refactor throttle timer" 2017-12-19 16:23:20 -06:00
Anton Kaliaev 70e30f74e6
Revert "Refactor repeat timer" 2017-12-19 16:16:16 -06:00
Emmanuel Odeke 8638961f02
common: Rand* warnings about cryptographic unsafety
Lesson articulated by @jaekwon on why we need 80 bits
of entropy at least before we can think of cryptographic
safety. math/rand's seed is a max of 64 bits so can never
be cryptographically secure.

Also added some benchmarks for RandBytes
2017-12-15 22:41:36 -07:00
Emmanuel Odeke cdc7988823
common: use genius simplification of tests from @ebuchman
Massive test simplication for more reliable tests from @ebuchman
2017-12-15 02:14:08 -07:00
Ethan Buchman b5f465b4ec common: use names prng and mrand 2017-12-15 00:23:25 -05:00
Emmanuel Odeke 29471d75cb
common: no more relying on math/rand.DefaultSource
Fixes https://github.com/tendermint/tmlibs/issues/99
Updates https://github.com/tendermint/tendermint/issues/973

Removed usages of math/rand.DefaultSource in favour of our
own source that's seeded with a completely random source
and is safe for use in concurrent in multiple goroutines.
Also extend some functionality that the stdlib exposes such as
* RandPerm
* RandIntn
* RandInt31
* RandInt63

Also added an integration test whose purpose is to be run as
a consistency check to ensure that our results never repeat
hence that our internal PRNG is uniquely seeded each time.
This integration test can be triggered by setting environment variable:
`TENDERMINT_INTEGRATION_TESTS=true`
for example
```shell
TENDERMINT_INTEGRATION_TESTS=true go test
```
2017-12-14 00:18:30 -07:00
Anton Kaliaev ff2fd63bf7
rename trySend to send 2017-12-08 11:17:07 -06:00
Ethan Frey ec4adf21e0 Cleanup from PR comments 2017-12-08 10:15:26 +01:00
Ethan Frey cc7a87e27c Use Ticker in Repeat again to avoid drift 2017-12-07 11:22:54 +01:00
Ethan Frey 8797197cdf No more blocking on multiple Stop() 2017-12-07 10:38:50 +01:00
Ethan Frey 887d766c86 Refactored RepeatTimer, tests hang 2017-12-07 10:15:38 +01:00
Anton Kaliaev 3779310c72
return back output internal channel (way go does with Timer) 2017-12-06 18:48:39 -06:00
Ethan Frey 8b518fadb2 Don't close throttle channel, explain why 2017-12-06 22:28:18 +01:00
Ethan Frey e430d3f844 One more attempt with a read-only channel 2017-12-06 21:51:23 +01:00
Ethan Frey 1ac4c5dd6d Made throttle output non-blocking 2017-12-06 21:20:30 +01:00
Ethan Frey 0a8721113a First pass of PR updates 2017-12-06 21:08:55 +01:00
Ethan Frey 4ec7883891 Cleanup 2017-12-06 11:21:01 +01:00
Ethan Frey dcb4395604 Refactor throttle timer 2017-12-06 11:17:50 +01:00
Ethan Frey 3d9113c16e Add a bit more padding to tests so they pass on osx with -race 2017-12-06 09:18:04 +01:00
Ethan Frey 26abd65e34 Add tests for repeat timer 2017-12-05 15:01:07 +01:00
Ethan Frey 53cdb6cf82 Demo throttle timer is broken 2017-12-05 14:49:16 +01:00
Anton Kaliaev 33abe87c5b
IntInSlice and StringInSlice functions
Refs https://github.com/tendermint/tendermint/pull/835
2017-11-29 12:18:03 -06:00
Ethan Buchman 4d991acae0 common: comments for Service 2017-11-29 05:16:15 +00:00
Anton Kaliaev c2fcc093b2
remove bool from Service#Reset 2017-11-27 23:42:36 -06:00
Anton Kaliaev e6164d4052
change service#Stop to be similar to Start 2017-11-06 12:47:23 -05:00
Anton Kaliaev 4123d54bf6
change service#Start to return just error (Refs #45)
```
@melekes
yeah, bool is superfluous
@ethanfrey
If I remember correctly when I was writing test code, if I call Start() on a Service that is already running, it returns (false, nil). Only if I try to legitimately start it, but it fails in startup do I get an error.
The distinction is quite important to make it safe for reentrant calls. The other approach would be to have a special error type like ErrAlreadyStarted, then check for that in your code explicitly. Kind of like if I make a db call in gorm, and get an error, I check if it is a RecordNotFound error, or whether there was a real error with the db query.
@melekes
Ah, I see. Thanks. I must say I like ErrAlreadyStarted approach more (not just in Golang)
```
2017-11-06 12:18:04 -05:00
Anton Kaliaev 49d75e223e
use os.Process#Kill (Fixes #73) 2017-11-04 08:14:47 -05:00
Anton Kaliaev b658294a13
use assert.Contains in cmap_test 2017-11-04 00:10:59 -05:00
Wolf 88481fc363 Make iterating over keys possible (#63)
* Make iterating over keys possible

* add test for cmap
- test Keys() and Values() respectively

* one cmap per test-case
2017-11-04 00:06:20 -05:00
Ethan Buchman 092eb701c7 cmn: Kill 2017-10-27 11:01:40 -04:00
Anton Kaliaev bcf15e527d
make GoPath a function
otherwise it could try to execute go binary and panic if no go binary
found. See https://github.com/tendermint/tendermint/issues/782
2017-10-25 11:01:52 +04:00
Anton Kaliaev 35e38e8932
call go env GOPATH if env var is not found (Refs #60) 2017-10-11 12:42:54 +04:00
Ethan Buchman 7dd6b3d3f8 Merge pull request #53 from tendermint/metalinter
add metalinter to CI and address some lint warnings
2017-10-04 00:21:24 -04:00
Ethan Buchman c8805fd7de metalinter fixes from review 2017-10-04 00:13:58 -04:00
Zach Ramsay cf49ba876f linter: couple fixes 2017-10-03 17:23:14 -04:00
Ethan Buchman 3d98504c4c common: WriteFileAtomic use tempfile in current dir 2017-09-22 13:20:13 -04:00
Zach Ramsay 3c57c24921 linting: next round of fixes 2017-09-22 12:14:27 -04:00
Zach Ramsay d6e03d2368 linting: add to Makefile & do some fixes 2017-09-22 11:42:29 -04:00
Ethan Buchman 9a2438e0dc common: Fingerprint comment 2017-09-20 02:49:51 -04:00
Ethan Buchman fe08fc00c8 Merge pull request #34 from orijtech/develop
common/IsDirEmpty: do not mask non-existance errors
2017-08-25 16:01:30 -04:00
Ethan Buchman 271145ee72 Merge pull request #32 from tendermint/bugfix/write-file-atomic
Fix rename /root/.tendermint_test/consensus_replay_test/priv_validator.json.new /root/.tendermint_test/consensus_replay_test/priv_validator.json: no such file or directory
2017-08-25 15:58:09 -04:00
Anton Kaliaev 956966e658
add missing validator package to glide.yaml 2017-08-11 16:36:26 -04:00
Emmanuel Odeke b4a51871b9
common/IsDirEmpty: do not mask non-existance errors
Currently IsDirEmpty returns true, err if it encounters
any error after trying to os.Open the directory.
I noticed this while studying the code and recalled a bug
from an earlier project in which doing the exact same thing
on code without permissions would trip out and falsely report
that the directory was empty.
Given demo.go in https://play.golang.org/p/vhTPU2RiCJ

* Demo:
```shell
$ mkdir -p sample-demo/1 && touch sample-demo/2
$ echo "1st round" && go run demo.go sample-demo
$ sudo chown root sample-demo && sudo chmod 0700 sample-demo
$ echo "2nd round" && go run demo.go sample-demo
```

That then prints out
```shell
1st round
original:: empty: false err: <nil>
updated::  empty: false err: <nil>
2nd round
original:: empty: true err: open data/: permission denied
updated::  empty: false err: open data/: permission denied
```

where in "2nd round", the original code falsely reports that
the directory is empty but that's a permission error.

I could write a code test for it, but that test requires me to change
users and switch to root as a Go user so no point in complicating our
tests, but otherwise it is a 1-to-1 translation between shell and Go.
2017-08-04 02:22:17 -06:00
Emmanuel Odeke d67a621715
http: http-utils added after extraction
Found common http utils that were being multiply duplicated across
many libraries and since am moving things in basecoin/unstable to
add for more functionality, it's better to put them in one
place.

Utilities and tests added:
- [X] FparseJSON
- [X] FparseAndValidateJSON
- [X] ParseRequestJSON
- [X] ParseAndValidateRequestJSON
- [X] WriteCode
- [X] WriteError
- [X] WriteSuccess
- [X] ErrorResponse

During review from @ethanfrey, made updates:
* Removed tt.want since it was a distraction/artifact that made
the reviewer think the tests weren't testing for both failed
and passed results.
* Added ErrorWithCode as WithCode is a common options pattern
in Go that could cause confusion:
  ErrorWithCode(error, int) ErrorResponse
* Using json.NewDecoder(io.Reader) error instead of
ioutil.ReadAll(io.Reader) to slurp all the bytes.
* Added more test scenarios to achieve 100% coverage of http.go
2017-08-02 11:38:52 -06:00
Anton Kaliaev d1ca2c6f83
[common] add a test for WriteFileAtomic 2017-07-28 11:40:21 -04:00
Anton Kaliaev b25aa3b472
[common] do not create {filePath}.bak in WriteFileAtomic
We use WriteFileAtomic in two places:

```
p2p/addrbook.go
338:    err = cmn.WriteFileAtomic(filePath, jsonBytes, 0644)

types/priv_validator.go
162:    err = WriteFileAtomic(privVal.filePath, jsonBytes, 0600)
```

and we don't need .bak in any of the above. We save priv_validator every
10ms and addrbook every 2 min.
2017-07-28 11:40:21 -04:00
Anton Kaliaev 8a51210efc
[common] use temp intead of {filePath}.new
The problem with {filePath}.new is that it is not safe for concurrent
use! Calling this function with the same params results in the following
error:

```
panic: Panicked on a Crisis: rename /root/.tendermint_test/consensus_replay_test/priv_validator.json.new /root/.tendermint_test/consensus_replay_test/priv_validator.json: no such file or directory

goroutine 47860 [running]:
github.com/tendermint/tendermint/vendor/github.com/tendermint/tmlibs/common.PanicCrisis(0xcba800, 0xc42152d640)
	/go/src/github.com/tendermint/tendermint/vendor/github.com/tendermint/tmlibs/common/errors.go:33 +0x10f
github.com/tendermint/tendermint/types.(*PrivValidator).save(0xc42235f2c0)
	/go/src/github.com/tendermint/tendermint/types/priv_validator.go:165 +0x159
github.com/tendermint/tendermint/types.(*PrivValidator).signBytesHRS(0xc42235f2c0, 0x6, 0x0, 0xc424e88f03, 0xc429908580, 0xca, 0x155, 0x80, 0xc424e88f00, 0x7f4ecafc88d0, ...)
	/go/src/github.com/tendermint/tendermint/types/priv_validator.go:249 +0x2bb
github.com/tendermint/tendermint/types.(*PrivValidator).SignVote(0xc42235f2c0, 0xc4228c7460, 0xf, 0xc424e88f00, 0x0, 0x0)
	/go/src/github.com/tendermint/tendermint/types/priv_validator.go:186 +0x1a2
github.com/tendermint/tendermint/consensus.(*ConsensusState).signVote(0xc424efd520, 0xc400000002, 0xc422d5e3c0, 0x14, 0x20, 0x1, 0xc4247b6560, 0x14, 0x20, 0x0, ...)
	github.com/tendermint/tendermint/consensus/_test/_obj_test/state.go:1556 +0x35e
github.com/tendermint/tendermint/consensus.(*ConsensusState).signAddVote(0xc424efd520, 0x2, 0xc422d5e3c0, 0x14, 0x20, 0x1, 0xc4247b6560, 0x14, 0x20, 0xc42001b300)
	github.com/tendermint/tendermint/consensus/_test/_obj_test/state.go:1568 +0x200
github.com/tendermint/tendermint/consensus.(*ConsensusState).enterPrecommit(0xc424efd520, 0x6, 0x0)
	github.com/tendermint/tendermint/consensus/_test/_obj_test/state.go:1082 +0x13a4
github.com/tendermint/tendermint/consensus.(*ConsensusState).addVote(0xc424efd520, 0xc424e88780, 0x0, 0x0, 0x39, 0x1dc, 0x28c)
	github.com/tendermint/tendermint/consensus/_test/_obj_test/state.go:1477 +0x1be5
github.com/tendermint/tendermint/consensus.(*ConsensusState).tryAddVote(0xc424efd520, 0xc424e88780, 0x0, 0x0, 0xd7fb00, 0xc42152ce00)
	github.com/tendermint/tendermint/consensus/_test/_obj_test/state.go:1382 +0x93
github.com/tendermint/tendermint/consensus.(*ConsensusState).handleMsg(0xc424efd520, 0xcb58e0, 0xc42547feb8, 0x0, 0x0, 0x6, 0x0, 0x4, 0xed10ca07e, 0x3077bfea, ...)
	github.com/tendermint/tendermint/consensus/_test/_obj_test/state.go:660 +0x9fc
github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine(0xc424efd520, 0x0)
	github.com/tendermint/tendermint/consensus/_test/_obj_test/state.go:615 +0x5f5
created by github.com/tendermint/tendermint/consensus.(*ConsensusState).OnStart
	github.com/tendermint/tendermint/consensus/_test/_obj_test/state.go:332 +0x4a7
exit status 2
FAIL	github.com/tendermint/tendermint/consensus	76.644s
make: *** [test_integrations] Error 1
```

See https://github.com/tendermint/tendermint/pull/568
2017-07-28 11:39:59 -04:00