Tentative of a cli template #54
Conversation
|
With nodes started using: |
cmd/mod.go
Outdated
|
|
||
| // Controller is the interface to implement to enable commands specific to | ||
| // the ledger implementation. | ||
| type Controller interface { |
There was a problem hiding this comment.
Per se the name "controller" is correct, but for me it has more the meaning of the link beween a model and its representation in an MVC pattern. Maybe just CLI ?
cmd/mod.go
Outdated
| Run(Context, Injector) error | ||
| } | ||
|
|
||
| // Request is a context to handle commands sent to the daemon. |
There was a problem hiding this comment.
"Request is a context": very confusing since there is exactly a "Context" struct bellow. So maybe update the comment or the Context struct? Because there is a lot of different part in this package it is hard to see what would be THE Context. So maybe Context should be RunContext or something like that.
cmd/mod.go
Outdated
| Out io.Writer | ||
| } | ||
|
|
||
| // Context is provide to an action to read the flags. |
There was a problem hiding this comment.
| // Context is provide to an action to read the flags. | |
| // Context is provided to an action to read the flags. |
cmd/mod.go
Outdated
| Execute(Request) error | ||
| } | ||
|
|
||
| // Builder defines how to build a list of commands to tweak the ledger. |
There was a problem hiding this comment.
"...to tweak the ledger" ?
EDIT: ok, didn't realize this abstraction was tight to a ledger configuration. I was under the impression that this abstraction would be totally independent and general-purpose.
cmd/app.go
Outdated
| } | ||
|
|
||
| // SocketDaemon is a daemon using UNIX socket. This allows the permissions to be | ||
| // manage by the filesystem. A user must have read/write access to send a |
There was a problem hiding this comment.
| // manage by the filesystem. A user must have read/write access to send a | |
| // managed by the filesystem. A user must have read/write access to send a |
mino/minogrpc/server.go
Outdated
|
|
||
| msg := &Certificate{Address: text, Value: cert} | ||
|
|
||
| // Prepare the list of send back to the new node. |
There was a problem hiding this comment.
it's too late for me to rephrase it, but it seems that a word should be changed.
mino/minogrpc/server.go
Outdated
|
|
||
| func (o overlayServer) Share(ctx context.Context, msg *Certificate) (*CertificateAck, error) { | ||
| // TODO: verify the validity of the certificate by connecting to the distant | ||
| // node by that requires a protection against malicious share. |
There was a problem hiding this comment.
| // node by that requires a protection against malicious share. | |
| // node but that requires a protection against malicious share. |
mino/minogrpc/tokens/mod.go
Outdated
| // Package tokens defines a token holder to generate and validate access tokens. | ||
| // and provides an implementation in-memory. |
There was a problem hiding this comment.
| // Package tokens defines a token holder to generate and validate access tokens. | |
| // and provides an implementation in-memory. | |
| // Package tokens defines a token holder to generate and validate access tokens | |
| // and provides an in-memory implementation. |
| } | ||
|
|
||
| // Verify implements tokens.Holder. It returns true if the token is valid. | ||
| func (holder *InMemoryHolder) Verify(token string) bool { |
There was a problem hiding this comment.
shouldn't the token also store the node address it has been issued to? Right now someone could potentially eave drop on the network, stole the token and use it. Verify would then be
Verify(token string, origin mino.Address)There was a problem hiding this comment.
Not sure where you can eavesdrop as the request is sent over TLS.
mino/minogrpc/server.go
Outdated
| @@ -319,6 +461,7 @@ func makeCertificate() (*tls.Certificate, error) { | |||
|
|
|||
| tmpl := &x509.Certificate{ | |||
| SerialNumber: big.NewInt(1), | |||
| DNSNames: []string{}, | |||
|
@nkcr Thanks a lot, I wasn't super happy with the abstraction but it now looks much better. I changed one thing from our discussion this morning which is the Action for a node builder that is now a ActionTemplate that you transformed into an actual action which allows to have the abstraction at the top level. |
Tentative of a cli template
This PR implements a tentative of a CLI template for the modules to follow to implement their own controllers that can be chosen before compiling an ledger application.
Also: implement a controller to create a network of nodes with grpc