consensus: save block metadata #646

pro-wh · 2024-02-22T00:45:38Z

proposer and signers

analyzer/consensus/registration.go

mitjat

Thank you thank you for figuring these out!
Generally LGTM. Most of my comments are about explaining the semantics to our API consumers (and to future nexus code maintainers).

storage/migrations/00_consensus.up.sql

analyzer/consensus/consensus.go

api/spec/v1.yaml

storage/client/client.go

api/spec/v1.yaml

storage/client/queries/queries.go

mitjat · 2024-03-12T06:28:01Z

go.mod

@@ -66,6 +65,7 @@ require (
 	github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect
 	github.com/go-git/go-billy/v5 v5.5.0 // indirect
 	github.com/go-git/go-git/v5 v5.11.0 // indirect
+	github.com/go-kit/kit v0.12.0 // indirect


Do you understand how this was added? We're not using it directly, and the only other depedendency change is that cometbft went from being an implicit dep to an explicit one. Maybe Go only adds indirect deps when they are actually used, rather than all transitive deps?

api/spec/v1.yaml

Andrew7234

Looks good! There's a few remaining comments from ptrus/mitja which need to be addressed, but hopefully those are minor changes. Thanks

analyzer/queries/queries.go

ptrus · 2024-10-01T07:45:01Z

storage/migrations/32_block_meta.up.sql

+
+ALTER TABLE chain.blocks
+    ADD COLUMN proposer_entity_id base64_ed25519_pubkey,
+    ADD COLUMN signer_entity_ids base64_ed25519_pubkey[];


What was the reasoning for storing Entity IDs instead of Node IDs here?

A validator entity can have multiple validator nodes registered. In that case, we lose information about which exact node signed the block.

In practice, only a single node per entity will be active at any given epoch (since the network limits to one at this point), so this shouldn't be that problematic. However, I feel we could easily have used node IDs here to avoid this.

Actually, it probably makes sense/is fine, since we mostly don't care about individual validator nodes in other places/in the API.

yeah, it's for reasons of API simplicity like that. the general style of nexus is to have analysis precompute stuff that the API side will need, even sometimes at the cost of lossiness

we mostly don't care about individual validator nodes in other places/in the API.

Yeah it was a combination of this, also because we don't track historical validator entity<>node mappings (though it is possible to add now in the history.validators table).

ptrus · 2024-10-10T13:03:16Z

storage/migrations/00_consensus.up.sql

-  block_hash HEX64 NOT NULL,
-  time       TIMESTAMP WITH TIME ZONE NOT NULL,
-  num_txs    UINT31 NOT NULL,
+  block_hash HEX64 NOT NULL,  -- NULL in 32_block_meta.up.sql


These comments are wrong 32_block_meta.up.sql does not make these fields NULL-able.

ptrus · 2024-10-10T13:09:25Z

analyzer/consensus/consensus.go

+			prevSigners = append(prevSigners, entity.String())
+		}
+		batch.Queue(
+			queries.ConsensusBlockAddSigners,


This doesn't work correctly in the fast-sync case. When in fast-sync, blocks are not processed in order, so the previous block (cmtMeta.LastCommit.Height) might not exist, and therefore this query won't insert any singers.

Some quick suggestion on how to fix this:

doing an Upsert, and making all other fields NULL-able (I think a previous version of this PR was done in that way)

storing the signers in a separate table

I noticed this in E2E tests of #764 since some blocks are missing signers in the edenfast test case.

pro-wh commented Feb 22, 2024

View reviewed changes

analyzer/consensus/registration.go Outdated Show resolved Hide resolved

pro-wh force-pushed the pro-wh/feature/genesisbatch branch from 92aa008 to 7af6e38 Compare February 23, 2024 01:29

pro-wh force-pushed the pro-wh/feature/blockmeta branch from 7e329ef to c38d8f2 Compare February 23, 2024 01:29

pro-wh force-pushed the pro-wh/feature/genesisbatch branch from 7af6e38 to faf3cbd Compare March 1, 2024 00:07

pro-wh force-pushed the pro-wh/feature/blockmeta branch 2 times, most recently from 09b62ae to 79bdda4 Compare March 4, 2024 21:15

pro-wh force-pushed the pro-wh/feature/genesisbatch branch 2 times, most recently from f67bc7b to 943a172 Compare March 6, 2024 00:36

pro-wh force-pushed the pro-wh/feature/blockmeta branch 2 times, most recently from ffeb35d to 04b1c2c Compare March 6, 2024 22:30

pro-wh marked this pull request as ready for review March 6, 2024 23:08

pro-wh requested review from Andrew7234, mitjat and ptrus as code owners March 6, 2024 23:08

pro-wh changed the title ~~(unfinished) consensus: save block metadata~~ consensus: save block metadata Mar 6, 2024

mitjat reviewed Mar 12, 2024

View reviewed changes

pro-wh force-pushed the pro-wh/feature/blockmeta branch from 04b1c2c to 627e4d0 Compare March 12, 2024 23:34

pro-wh force-pushed the pro-wh/feature/genesisbatch branch 2 times, most recently from 8457396 to 1e6aa7e Compare March 18, 2024 23:16

pro-wh force-pushed the pro-wh/feature/blockmeta branch 2 times, most recently from 003826c to 07fc1af Compare March 18, 2024 23:18

pro-wh force-pushed the pro-wh/feature/genesisbatch branch from b44e84b to 2487e50 Compare March 21, 2024 22:53

pro-wh force-pushed the pro-wh/feature/blockmeta branch 3 times, most recently from 640bdb7 to 89331f8 Compare March 26, 2024 23:00

pro-wh force-pushed the pro-wh/feature/genesisbatch branch 2 times, most recently from 944e7a8 to 24cbe24 Compare March 27, 2024 23:41

pro-wh force-pushed the pro-wh/feature/blockmeta branch from 89331f8 to d1f3ce3 Compare March 27, 2024 23:41

pro-wh force-pushed the pro-wh/feature/genesisbatch branch from 24cbe24 to 8a8fbb1 Compare March 29, 2024 22:53

pro-wh force-pushed the pro-wh/feature/blockmeta branch from d1f3ce3 to 5e9bbf1 Compare March 29, 2024 22:53

jberci force-pushed the pro-wh/feature/blockmeta branch 2 times, most recently from dee5dc1 to e0ea8d8 Compare August 30, 2024 21:18

Andrew7234 approved these changes Sep 3, 2024

View reviewed changes

analyzer/queries/queries.go Outdated Show resolved Hide resolved

jberci force-pushed the pro-wh/feature/blockmeta branch from 0f3b539 to aa9edaf Compare September 4, 2024 14:44

pro-wh and others added 12 commits September 12, 2024 18:24

storage: add node consensus pubkey address and index

80d2d56

consensus: index node consensus pubkey address

880eea0

storage: add block metadata fields

b3e4f4c

consensus: save block metadata

889eb6e

api: add consensus block proposer and signers specs

7e97926

storage: reformat client Blocks query

9b1d6a0

storage: query block proposer and signers

d6fe627

add changelog

5a66108

tests/e2e: update expected responses

947e22e

storage/client: make entity order consistent

76d237a

consensus: store entity ids instead of consensus addresses

7162278

tests: update damask e2e

0ba038a

jberci force-pushed the pro-wh/feature/blockmeta branch from aa9edaf to 0ba038a Compare September 12, 2024 16:47

jberci merged commit 68bb393 into main Sep 16, 2024
16 checks passed

jberci deleted the pro-wh/feature/blockmeta branch September 16, 2024 16:42

ptrus reviewed Oct 1, 2024

View reviewed changes

ptrus reviewed Oct 10, 2024

View reviewed changes

buberdds mentioned this pull request Oct 15, 2024

Enable proposer column in Consensus block list oasisprotocol/explorer#1573

Merged

pro-wh mentioned this pull request Oct 23, 2024

Update block proposer API #788

Open

buberdds mentioned this pull request Oct 31, 2024

Some blocks are missing signers #795

Open

tjanez mentioned this pull request Dec 3, 2024

api/consensus: support filtering blocks by proposer #764

Merged

ptrus mentioned this pull request Dec 20, 2024

analyzer: Get information about block proposers and signatures #74

Closed

This was referenced Dec 22, 2024

Validator list #859

Closed

Track liveness of validator nodes/entities #860

Open

Validator details #861

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

consensus: save block metadata #646

consensus: save block metadata #646

pro-wh commented Feb 22, 2024 •

edited

Loading

mitjat left a comment

mitjat Mar 12, 2024

Andrew7234 left a comment

ptrus Oct 1, 2024 •

edited

Loading

ptrus Oct 1, 2024

pro-wh Oct 1, 2024

Andrew7234 Oct 1, 2024

ptrus Oct 10, 2024

ptrus Oct 10, 2024

consensus: save block metadata #646

consensus: save block metadata #646

Conversation

pro-wh commented Feb 22, 2024 • edited Loading

mitjat left a comment

Choose a reason for hiding this comment

mitjat Mar 12, 2024

Choose a reason for hiding this comment

Andrew7234 left a comment

Choose a reason for hiding this comment

ptrus Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

ptrus Oct 1, 2024

Choose a reason for hiding this comment

pro-wh Oct 1, 2024

Choose a reason for hiding this comment

Andrew7234 Oct 1, 2024

Choose a reason for hiding this comment

ptrus Oct 10, 2024

Choose a reason for hiding this comment

ptrus Oct 10, 2024

Choose a reason for hiding this comment

pro-wh commented Feb 22, 2024 •

edited

Loading

ptrus Oct 1, 2024 •

edited

Loading