Initial sentry node support #2300

tjanez · 2019-10-24T10:57:40Z

No description provided.

go/oasis-test-runner/oasis/args.go

ravenac95 · 2019-10-31T19:37:18Z

go/common/node/address.go

+// NOTE: This could be the P2P ID and IP of a sentry node.
+type ConsensusAddress struct {
+	ID      signature.PublicKey `json:"id"`
+	Address Address             `json:"address"`


General question...

Unless I'm mistake, the tendermint protocol sentry nodes as used for cosmos are meant to be a dynamic set of machines. These sentry node machines could in theory continuously disappear and reappear with different IP addresses and identities. The gossip nature of the tendermint protocol would allow this to work. Is there a reason that we need to specify more than the ID when registering a node for consensus?

cc @Yawning @kostko

Yeah this would probably work as well, but would rely exclusively on the seed nodes to establish peers via PEX.

Also tendermint AFAIK requires you to specify validator addresses in the genesis block?

Also "sentry nodes" (aka proxies) for other things (gRPC and possibly libp2p) will definitely require addresses and we should keep that in mind while designing any proxy stuff.

Also tendermint AFAIK requires you to specify validator addresses in the genesis block?

Strictly speaking, no: https://godoc.org/github.com/tendermint/tendermint/types#GenesisValidator

Our tendermint seed node implementation currently uses the genesis document to populate the initial address book. This can be omitted, assuming "enough" nodes use the seed node. Time to a functional network after redeploy will be longer.

From a practical perspective, we use more than tendermint, and I don't see us implementing a gossip protocol anytime soon. So I'm not sure what this saves if anything.

As long as it's possible for the tendermint sentry node to disappear entirely I would say whatever gets registered is alright.

Also "sentry nodes" (aka proxies) for other things (gRPC and possibly libp2p) will definitely require addresses and we should keep that in mind while designing any proxy stuff.

I would say registering an address for libp2p/grpc proxy would be fine. Would it be possible to register a DNS name and cert for those proxies so that IPs can float and be scaled out?

Would it be possible to register a DNS name and cert for those proxies so that IPs can float and be scaled out?

Yes.

codecov · 2019-11-04T14:09:06Z

Codecov Report

Merging #2300 into master will increase coverage by 0.17%.
The diff coverage is 65.18%.

@@            Coverage Diff             @@
##           master    #2300      +/-   ##
==========================================
+ Coverage   66.17%   66.35%   +0.17%     
==========================================
  Files         288      292       +4     
  Lines       29444    29663     +219     
==========================================
+ Hits        19484    19682     +198     
- Misses       7620     7627       +7     
- Partials     2340     2354      +14

Impacted Files	Coverage Δ
go/consensus/tendermint/api/api.go	`54.92% <ø> (ø)`	⬆️
go/consensus/tendermint/tendermint.go	`64.16% <100%> (+0.09%)`	⬆️
go/worker/registration/worker.go	`68.79% <41.17%> (-1.33%)`	⬇️
go/oasis-node/cmd/registry/node/node.go	`35.71% <46.66%> (+0.3%)`	⬆️
go/oasis-node/cmd/node/node.go	`55.58% <52.08%> (-0.56%)`	⬇️
go/worker/common/config.go	`66.21% <64.28%> (-3.79%)`	⬇️
go/sentry/client/client.go	`72.54% <72.54%> (ø)`
go/sentry/sentry.go	`73.33% <73.33%> (ø)`
go/worker/sentry/worker.go	`76.78% <76.78%> (ø)`
go/sentry/grpc.go	`81.81% <81.81%> (ø)`
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f6d2de...a8b2c2e. Read the comment docs.

go/oasis-test-runner/oasis/fixture.go

go/oasis-test-runner/oasis/validator.go

ptrus · 2019-11-07T10:52:50Z

go/tendermint/tendermint.go

 	tenderConfig.P2P.SeedMode = viper.GetBool(CfgP2PSeedMode)
 	// Seed Ids need to be Lowecase as p2p/transport.go:MultiplexTransport.upgrade()
-	// uses a case sensitive string comparision to validate public keys
+	// uses a case sensitive string comparision to validate public keys.
 	// Since Seeds is expected to be in comma-delimited id@host:port format,
 	// lowercasing the whole string is ok.
 	tenderConfig.P2P.Seeds = strings.ToLower(viper.GetString(CfgP2PSeeds))


I there there were some plans in making CfgP2PSeeds a string slice (on our end) instead of comma-delimited list, maybe a good time to change it

I've also decided to make a breaking configuration change here and renamed tendermint.seeds configuraiton flag to tendermint.seed to be consistent with newly added tendermint.*peer options.

@ptrus, please take a look.

go/tendermint/tendermint.go

go/sentry/sentry.go

go/sentry/client/client.go

go/sentry/grpc.go

kostko

This looks good to me, just fix the ctx and then it's good to merge!

go/sentry/sentry.go

Support passing multiple consensus addresses via cfgConsensusAddress.

When registering a validator, query sentry nodes for their consensus address(es) if sentry nodes are configured.

Rename tendermint.seeds configuraiton flag to tendermint.seed to be consistent with tendermint.*peer options. Change the configuration flag to accept a string slice on our side to make the seed node configuration easier.

Yawning reviewed Oct 25, 2019

View reviewed changes

go/oasis-test-runner/oasis/args.go Outdated Show resolved Hide resolved

go/oasis-test-runner/oasis/args.go Outdated Show resolved Hide resolved

tjanez force-pushed the tjanez/add-sentry-support branch 2 times, most recently from ab6bed9 to 952ab7c Compare October 29, 2019 15:51

tjanez mentioned this pull request Oct 30, 2019

Miscellaneous improvements #2313

Merged

tjanez force-pushed the tjanez/add-sentry-support branch 3 times, most recently from 44562ee to 2c8391c Compare October 31, 2019 15:55

ravenac95 reviewed Oct 31, 2019

View reviewed changes

tjanez force-pushed the tjanez/add-sentry-support branch from 2c8391c to b31cd70 Compare November 4, 2019 13:58

tjanez mentioned this pull request Nov 4, 2019

Generalize ConsensusInfo's addresses #2328

Merged

tjanez force-pushed the tjanez/add-sentry-support branch 6 times, most recently from dbc8d02 to 2802735 Compare November 6, 2019 10:16

tjanez marked this pull request as ready for review November 6, 2019 10:17

tjanez requested a review from peterjgilbert as a code owner November 6, 2019 10:17

tjanez force-pushed the tjanez/add-sentry-support branch 2 times, most recently from fc84120 to ba3031c Compare November 6, 2019 16:35

kostko reviewed Nov 6, 2019

View reviewed changes

go/oasis-test-runner/oasis/fixture.go Show resolved Hide resolved

go/oasis-test-runner/oasis/fixture.go Outdated Show resolved Hide resolved

ptrus reviewed Nov 7, 2019

View reviewed changes

go/oasis-test-runner/oasis/validator.go Outdated Show resolved Hide resolved

ptrus reviewed Nov 7, 2019

View reviewed changes

go/tendermint/tendermint.go Outdated Show resolved Hide resolved

tjanez mentioned this pull request Nov 7, 2019

go/oasis-node/cmd/registry/node: Make consensus address' ID@ optional #2345

Merged

tjanez force-pushed the tjanez/add-sentry-support branch 4 times, most recently from 8d3fd26 to 5fe3359 Compare November 8, 2019 14:31

tjanez force-pushed the tjanez/add-sentry-support branch 4 times, most recently from 580a6ed to 96124b9 Compare November 13, 2019 13:56

kostko reviewed Nov 13, 2019

View reviewed changes

go/sentry/sentry.go Outdated Show resolved Hide resolved

go/sentry/sentry.go Outdated Show resolved Hide resolved

kostko reviewed Nov 13, 2019

View reviewed changes

go/sentry/client/client.go Outdated Show resolved Hide resolved

go/sentry/client/client.go Outdated Show resolved Hide resolved

go/sentry/client/client.go Show resolved Hide resolved

go/sentry/grpc.go Outdated Show resolved Hide resolved

tjanez force-pushed the tjanez/add-sentry-support branch 2 times, most recently from 459578f to 7d04d1d Compare November 13, 2019 15:02

tjanez added the c:breaking/cfg Category: breaks configuration label Nov 13, 2019

kostko approved these changes Nov 13, 2019

View reviewed changes

go/sentry/sentry.go Outdated Show resolved Hide resolved

ptrus approved these changes Nov 13, 2019

View reviewed changes

ptrus mentioned this pull request Nov 13, 2019

Merge latest docs from private operator docs oasisprotocol/old-docs#107

Merged

go/oasis-node/cmd/registry/node: Support multiple consensus addresses

820a864

Support passing multiple consensus addresses via cfgConsensusAddress.

tjanez force-pushed the tjanez/add-sentry-support branch 2 times, most recently from 0c11c6d to b6a62c5 Compare November 13, 2019 21:19

tjanez added 11 commits November 13, 2019 22:22

go/sentry: Initial API, backend and gRPC implementation

73eca14

go/worker/common: Add support for configuring Sentry nodes

c2f549c

go/worker/sentry: Initial Sentry worker implementation

fbc1398

go/oasis-node/cmd/node: Add support for launching Sentry nodes

09d7df0

go/oasis-node/cmd/node: Tidy up worker initialization and start up

59134df

go/worker/registration: Support querying sentries for consensus address

ca9759d

When registering a validator, query sentry nodes for their consensus address(es) if sentry nodes are configured.

go/consensus/tendermint: Add config for private & persistent peers, Pex

838892b

go/consensus/tendermint: Rename seed configuration flag

37f5203

Rename tendermint.seeds configuraiton flag to tendermint.seed to be consistent with tendermint.*peer options. Change the configuration flag to accept a string slice on our side to make the seed node configuration easier.

go/consensus/tendermint/api: Add LogEventPeerExchangeDisabled

d292c0e

go/oasis-test-runner: Add sentry node support

fac9e4e

go/oasis-test-runner: Add sentry E2E tests scenario

a8b2c2e

tjanez force-pushed the tjanez/add-sentry-support branch from b6a62c5 to a8b2c2e Compare November 13, 2019 21:24

tjanez merged commit e14f636 into master Nov 14, 2019

tjanez mentioned this pull request Nov 14, 2019

Enable Tendermint Protocol Sentry in Ekiden Node #1828

Closed

tjanez deleted the tjanez/add-sentry-support branch November 14, 2019 08:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial sentry node support #2300

Initial sentry node support #2300

tjanez commented Oct 24, 2019

ravenac95 Oct 31, 2019

kostko Oct 31, 2019

kostko Oct 31, 2019

Yawning Nov 4, 2019

ravenac95 Nov 4, 2019 •

edited

Loading

kostko Nov 4, 2019

codecov bot commented Nov 4, 2019 •

edited

Loading

ptrus Nov 7, 2019

tjanez Nov 13, 2019

tjanez Nov 13, 2019

kostko left a comment

Initial sentry node support #2300

Initial sentry node support #2300

Conversation

tjanez commented Oct 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ravenac95 Nov 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 4, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kostko left a comment

Choose a reason for hiding this comment

ravenac95 Nov 4, 2019 •

edited

Loading

codecov bot commented Nov 4, 2019 •

edited

Loading