netsec-ethz / scion-apps

Public repository for SCION applications
Apache License 2.0
20 stars 43 forks source link

hellodrkey example #177

Closed juagargi closed 3 years ago

juagargi commented 3 years ago

This change is Reviewable

marcfrei commented 3 years ago

_examples/hellodrkey/hellodrkey.go, line 54 at r1 (raw file):

func NewClient(sciondPath string) Client {
  sciond, err := sciond.NewService(sciondPath).Connect(context.Background())

Remove this empty line or add one in NewServer for consistency?

marcfrei commented 3 years ago

_examples/hellodrkey/hellodrkey.go, line 126 at r1 (raw file):

}

func (s Server) HostKey(meta drkey.Lvl2Meta) drkey.Lvl2Key {

Not used, right? Maybe remove this function to keep the example minimal?

marcfrei commented 3 years ago

_examples/hellodrkey/hellodrkey.go, line 151 at r1 (raw file):

  clientKey = client.HostKey(metaClient)

  durationClient := time.Since(t0)

Move this statement before the empty line?

marcfrei commented 3 years ago

_examples/hellodrkey/hellodrkey.go, line 29 at r1 (raw file):

)

var now = time.Now().UTC()

I didn't quite get why this is global var. Could we also keep it local to main and pass it around via function params?

marcfrei commented 3 years ago

a discussion (no related file): I like the example because it is really minimal.

I already know the DRKey system a little bit from Lightning Filter and had no problem to understand what is going on here. Users with no prior knowledge will obviously have a harder time to understand the motivation for this example. What would be the best reference to get them started?


marcfrei commented 3 years ago

_examples/hellodrkey/README.md, line 22 at r2 (raw file):


Both slow and fast paths should obtain the same key.
And both slow and fast path are measure for performance and their times displayed at the end.

"measured" instead of "measure"?

marcfrei commented 3 years ago

a discussion (no related file):

Previously, juagargi (Juan A. Garcia Pardo) wrote…
Thanks for the review. Indeed much more documentation was needed. I followed your advice and added the README with a description of what `hellodrkey` does, and how to configure the `gen` directory for it to run.

Thanks, Juan. I think the new README is very helpful.


marcfrei commented 3 years ago

_examples/hellodrkey/hellodrkey.go, line 34 at r2 (raw file):

)

// should return the same timestamp everytime that it's called, but for simplicity we just

Maybe "Should" instead of "should"?

marcfrei commented 3 years ago

_examples/hellodrkey/README.md, line 30 at r3 (raw file):


1. Create a local topology with the `tiny.topo` description: `./scion.sh topology -c ./topology/tiny.topo`.
1. Edit the CS configuration for `ff00:0:111` under `gen/ff00_0_111/cs1-ff00_0_111-1.toml`

gen/ASff00_0_111/cs1-ff00_0_111-1.toml

marcfrei commented 3 years ago

_examples/hellodrkey/README.md, line 48 at r3 (raw file):

   connection = "gen-cache/sd1-ff00_0_111.drkey.db"
  1. Edit the configuration for the CS in gen/ff00_0_112/cs1-ff00_0_112-1.toml and add:

gen/ASff00_0_112/cs1-ff00_0_112-1.toml

marcfrei commented 3 years ago
:lgtm:
marcfrei commented 3 years ago

I've tested the DRKey example with the configuration described in the new README: works perfectly -- just two small typos see below. Thank you very much!

marcfrei commented 3 years ago
:lgtm: