implment all 3 encoded connection id propoals

[da2ce7]

@josecelano @WarmBeer

I have written an adapted version of https://github.com/torrust/torrust-tracker/pull/60

• [X] Implemented all three types of encoded connection ID.
• [X] Implemented a static secret store (populated on start), for both the Blowfish and Server Secret.
• [X] Mocked Time and Secrets for tests.
• [X] Basic Benchmark of different types of encoded id.
• [X] Better Test coverage. Test Timeouts, etc.
• [ ] Make connection ID lifetime configurable.

It looks like the encrypted ID is the fastest to verify.

benchmark_make_old_unencoded_id                                                                                     
                        time:   [0.0000 ps 0.0000 ps 0.0000 ps]
Found 13 outliers among 100 measurements (13.00%)
  5 (5.00%) high mild
  8 (8.00%) high severe

benchmark_make_hashed_encoded_id                                                                            
                        time:   [243.08 ns 243.61 ns 244.18 ns]
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

benchmark_make_witness_encoded_id                                                                            
                        time:   [261.94 ns 262.49 ns 263.11 ns]

benchmark_make_encrypted_encoded_id                                                                            
                        time:   [65.045 ns 65.123 ns 65.200 ns]

[josecelano]

Cool @da2ce7!

I like having real speed metrics.

Do you want to include all the options in the final version or just record why we choose one of them?

[da2ce7]

@josecelano rebased upon your latest version. :)

[da2ce7]

hi @da2ce7

Hello @josecelano

I think we should not include the code we are not going to use and keep only the fastest implementation. We could keep this implementation in a different repo or branch to track why we use the encrypted version.

I completely agree, I plan to remove all the other implementations. The reason that I have included all three implementations was:

1. To compare the implementations.
2. To see what one is the most complex, what one is the fastest, etc.
3. To see the problem from a higher-level.
4. To help me design for what sort of abstractions make sense.

In general, I like the simplicity of the solution at the higher level. Still, having less abstraction forces us to see more Rust scaffolding code (for type casting) that leads to less readability, in my honest opinion. And also to less testability.

Yes I removed almost all the abstractions that you provided, as I'm still learning rust, I wish to use the standard library as much as possible. I do not want to re-invent the wheel.

I would be happy with a similar approach but removing test code from production code and making it easier to mock global values when needed, with different values, instead of hardcoded fixed values.

This too I agree with. I'm not sure what sort of abstractions make the best sense yet. I plan to look into the facade design pattern as you recommended.

[josecelano]

Yes I removed almost all the abstractions that you provided, as I'm still learning rust, I wish to use the standard library as much as possible. I do not want to re-invent the wheel.

Yes, I think that's something I'm still miserably failing. Specially with std::convert::Into trait:

"A value-to-value conversion that consumes the input value". https://doc.rust-lang.org/std/convert/trait.Into.html

I need time to get used to those self-killing objects. I have been discussing with @WarmBeer here.

[mickvandijke]

Very nice to see a comparison of all 3 implementations!

[da2ce7]

@josecelano I have reimplemented the clock, now it takes the form a Duration, and has Into and From traits implemented for both a local clock, and a fixed clock.

[josecelano]

@josecelano I have reimplemented the clock, now it takes the form a Duration, and has Into and From traits implemented for both a local clock, and a fixed clock.

I will take a look. I've also been thinking about how to keep the global variables in your functions without having two different implementations for tests and production code. I mean things like this:

if cfg!(test) { &TEST_BLOWFISH_KEY } else { &BLOWFISH_KEY };

I found this solution.

Which is actually an old solution. Solution 2 in this article.

The "facade" solution I mentioned is probably the more complex one.

[da2ce7]

@josecelano and @WarmBeer

I'm now felling that my version of the Clock Module is quite rusty: https://github.com/da2ce7/torrust-tracker/blob/protocol-tests/src/protocol/clock.rs

What do you think?

[josecelano]

@josecelano and @WarmBeer

I'm now felling that my version of the Clock Module is quite rusty: https://github.com/da2ce7/torrust-tracker/blob/protocol-tests/src/protocol/clock.rs

What do you think?

@da2ce7 yes, it's much rustier. I've created a PR with my feedback.

[da2ce7]

Requires rebase upon: #85

[josecelano]

hi @da2ce7 are you planning to merge this PR?

In general, I'm not fun of adding code that is not used in production but:

• We also have tests that are not used in production.
• We can consider it as documentation. In the future, we will know exactly why we are not using the other alternatives. It's also documentation for the UDP protocol.
• We could add an option to settings to use a different one. That would increase security because you do not even know which one the server is using.

[da2ce7]

Closing, current implementation is good for the first release.