-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Shutter service #560
base: main
Are you sure you want to change the base?
Shutter service #560
Conversation
Test/registry event
Test/newblock dec trigger
9225ac9
to
d86f29f
Compare
d86f29f
to
85e4578
Compare
930cb54
to
f6dc1a2
Compare
8e41380
to
12c2328
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks quite good.
Some comments below.
There's a lot of code duplication but as we discussed before that's fine for now.
I've added a session for our retreat to discuss removing duplication.
block_number bigint NOT NULL CHECK (block_number >= 0) | ||
); | ||
|
||
CREATE TABLE current_decryption_trigger( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite understand why it's "current" decryption trigger?
They don't seem to get deleted, wouldn't just decryption_trigger
make more sense?
@@ -0,0 +1,53 @@ | |||
package shutterservice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other keyper implementations all have keyper
in their command name. I think we should keep that convention.
"github.com/shutter-network/rolling-shutter/rolling-shutter/medley/metricsserver" | ||
"github.com/shutter-network/rolling-shutter/rolling-shutter/p2p" | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is missing an assignment check to verify the structs actually satisfy the interface. See for example:
rolling-shutter/rolling-shutter/keyperimpl/gnosis/config.go
Lines 21 to 25 in bae90ac
var ( | |
_ configuration.Config = &Config{} | |
_ configuration.Config = &GnosisConfig{} | |
_ configuration.Config = &GnosisContractsConfig{} | |
) |
var Definition db.Definition | ||
|
||
func init() { | ||
def, err := db.NewSQLCDefinition(files, "sql/", "shutterserviceKeyper", 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: In the gnosis impl keyper
is lower case.
No description provided.