Generalize Modules to 2 types: ActiveModule and PassiveModule #98

matejpavlovic · 2022-06-14T15:37:11Z

This is a very big PR, but each commit should be rather easily digestible.

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

pkg/clients/signingclienttracker.go

pkg/modules/nullmodule.go

pkg/modules/modules.go

node.go

workers.go

pkg/iss/iss.go

matejpavlovic · 2022-06-15T10:24:13Z

I think I addressed all the comments with what I've just pushed. Pls have a look and resolve the conversations if you are happy with the changes. Thanks!

pkg/modules/eventprocessing.go

xosmig · 2022-06-15T11:19:39Z

And what about Status()? Have you decided to keep it out of the Module interface?

pkg/clients/signingclienttracker.go

matejpavlovic · 2022-06-15T11:44:26Z

And what about Status()? Have you decided to keep it out of the Module interface?

Oh yes, I forgot about that. Fixed now.

matejpavlovic · 2022-06-15T11:48:28Z

pkg/modules/eventprocessing.go

+	// Start one concurrent worker for each event in the input list.
+	// Note that the processing starts concurrently for all events, and only the writing of the results is synchronized.
+	iter := eventsIn.Iterator()
+	i := 0


The i variable? It is, as each goroutine needs to know the index of its output channels and the Iterator implementation does not provide that information.

xosmig · 2022-06-15T11:52:38Z

LGTM

sergefdrv

That is a massive PR! Good job 👍

pkg/clients/signingclienttracker.go

protos/eventpb/eventpb.proto

pkg/deploytest/fakeapp.go

pkg/simplewal/simplewal.go

pkg/iss/iss.go

node.go

mir_test.go

pkg/deploytest/deployment.go

pkg/modules/modules.go

matejpavlovic · 2022-06-16T08:06:04Z

Ok I think I addressed all the stuff from above that required changes, plus I fixed the problem with the debugger not working.
Please let me know if I forgot anything, otherwise pls resolve the conversations so I can merge the PR. Thanks both for the improvements!

pkg/clients/signingclienttracker.go

pkg/crypto/api.go

pkg/iss/iss.go

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Use an explicit definition of the ImplementsModule function in each Module implementation instead of including the Module interface as an anonymous field. This smells less like abusing OO concepts in a non-OO language. Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

The foreign module names in ISS, currently defined as package-level variables, are only a temporary measure until these values become part of the ISS protocol configuration data structure. Until then, they stay "hard-coded" this way. Also, the OwnModuleName is renamed to issModuleName (instead of ownModuleName) Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

More concretely, in node.go and pkg/deploytest/deployment.go Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Remove the "key-value store"-like methods for public key management. If the crypto implementation needs to store public key information, it should manage the public keys out-of-band. Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

matejpavlovic added 7 commits June 13, 2022 18:16

Use dynamic reflect.Select in main event loop

15aca91

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Add dest_module field to event proto definitions

1305f75

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Add generic (passive) modules

8d14376

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Rename hash to digest in crypto implementation

4f18010

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Move default ISS module definitions to ISS package

9235d97

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Generalize Hasher module

f970b23

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Generalize App module

31fae4c

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

matejpavlovic requested review from sergefdrv and xosmig June 14, 2022 15:37

xosmig reviewed Jun 14, 2022

View reviewed changes

xosmig reviewed Jun 15, 2022

View reviewed changes

pkg/modules/eventprocessing.go Outdated Show resolved Hide resolved

pkg/modules/eventprocessing.go Show resolved Hide resolved

xosmig reviewed Jun 15, 2022

View reviewed changes

pkg/clients/signingclienttracker.go Outdated Show resolved Hide resolved

matejpavlovic force-pushed the general-modules branch from 8ab2819 to d001a6a Compare June 15, 2022 11:43

matejpavlovic commented Jun 15, 2022

View reviewed changes

sergefdrv reviewed Jun 15, 2022

View reviewed changes

matejpavlovic force-pushed the general-modules branch from d001a6a to a5f7fd0 Compare June 15, 2022 21:30

sergefdrv reviewed Jun 16, 2022

View reviewed changes

pkg/clients/signingclienttracker.go Outdated Show resolved Hide resolved

sergefdrv reviewed Jun 16, 2022

View reviewed changes

pkg/crypto/api.go Outdated Show resolved Hide resolved

sergefdrv reviewed Jun 16, 2022

View reviewed changes

pkg/iss/iss.go Outdated Show resolved Hide resolved

sergefdrv reviewed Jun 16, 2022

View reviewed changes

pkg/iss/iss.go Outdated Show resolved Hide resolved

matejpavlovic force-pushed the general-modules branch from 7babfef to 64c0c50 Compare June 16, 2022 10:53

Generalize Crypto module

8cee9a0

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

matejpavlovic force-pushed the general-modules branch 2 times, most recently from 9e6bde8 to 0cb26c7 Compare June 16, 2022 14:01

Generalize WAL module

83294f7

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

matejpavlovic added 25 commits June 16, 2022 16:12

Generalize ClientTracker module

8932949

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Generalize ReqestStore module

6cb41cd

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Generalize ISS protocol module

eba24ed

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Separate event interceptor from modules

512bd37

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Introduce the ActiveModule

18fc7ca

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Generalize Timer module

fcc6907

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Generalize Net module

7e8fef0

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Simplify and clean up event and module handling

2c934b9

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Remove unused dummy components

67415d8

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Remove old module definitions

3ba5cbc

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Abstract foreign module names in ISS

f4dcd0a

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Fix tests hanging on error

908acf9

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Add Null module and use it as default ISS WAL

7001729

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Update chat demo app to use new modules

0237ef4

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Remove obsolete NullNet and make linter happy

9ac48c3

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Sequential and concurrent event processing

416dfee

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Safely apply events also to ActiveModules

706c296

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Fix documentation of processModuleEvents

bd30747

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Move Status() function modules to Module

1819a8b

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Remove node status and ImplementsModule() panics

edb1c99

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Fix and simplify some select statements

f434280

More concretely, in node.go and pkg/deploytest/deployment.go Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Fix bug in stripping events, making debugging work

23ba462

Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

Simplify crypto implementation interface

c2f001f

Remove the "key-value store"-like methods for public key management. If the crypto implementation needs to store public key information, it should manage the public keys out-of-band. Signed-off-by: Matej Pavlovic <matopavlovic@gmail.com>

matejpavlovic force-pushed the general-modules branch from 0cb26c7 to c2f001f Compare June 16, 2022 14:13

matejpavlovic merged commit d696175 into consensus-shipyard:main Jun 16, 2022

matejpavlovic mentioned this pull request Jun 20, 2022

🚧 | Y3: Efficient subnet consensus protocols consensus-shipyard/consensuslab#9

Closed

matejpavlovic deleted the general-modules branch July 6, 2023 15:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generalize Modules to 2 types: ActiveModule and PassiveModule #98

Generalize Modules to 2 types: ActiveModule and PassiveModule #98

matejpavlovic commented Jun 14, 2022 •

edited

Loading

matejpavlovic commented Jun 15, 2022

xosmig commented Jun 15, 2022 •

edited

Loading

matejpavlovic commented Jun 15, 2022

matejpavlovic Jun 15, 2022

xosmig commented Jun 15, 2022

sergefdrv left a comment

matejpavlovic commented Jun 16, 2022

Generalize Modules to 2 types: ActiveModule and PassiveModule #98

Generalize Modules to 2 types: ActiveModule and PassiveModule #98

Conversation

matejpavlovic commented Jun 14, 2022 • edited Loading

matejpavlovic commented Jun 15, 2022

xosmig commented Jun 15, 2022 • edited Loading

matejpavlovic commented Jun 15, 2022

matejpavlovic Jun 15, 2022

Choose a reason for hiding this comment

xosmig commented Jun 15, 2022

sergefdrv left a comment

Choose a reason for hiding this comment

matejpavlovic commented Jun 16, 2022

matejpavlovic commented Jun 14, 2022 •

edited

Loading

xosmig commented Jun 15, 2022 •

edited

Loading