RA Learning using RALib

[Kax-y]

Finally got the code to merge properly with help from Oskar so here it is.

There are some failing checks which arose during this refactor that need to be investigated.

[kostis]

First of all, thanks @Kax-y for picking up this again. Let us know how/if you want help in any concrete way.

As @actyp mentioned, recently, the code of EDHOC-Fuzzer (and the CI) was updated to use Java 21. Due to this, I had to drop support for SIFIS-HOME which uses an ancient Java version and is no longer maintained. So, you can ignore these failures. Better yet, you can simply rebase on top of current main.

Other things that I would suggest are:

1. Instead of having CI tests that try to test both Mealy machine learning and RA learning in one go, to separate these. This will double the tests alright, and we will be downloading and installing the SULs twice, but it will allow us to have faster indications that the Mealy machine tests for all SULs are not broken.
2. Also, to speed up things, perhaps the CI can use less learning rounds for RA learning. This is not ideal, of course, but the (main) reason for the CI is to check that things are not super-broken, not to do exhaustive testing.

Keep up the good work and thanks again!

[kostis]

It's your PR, but time limits are problematic; random fluctuations (or even changes in the VMs where these tests run) can cause (surprising) differences. I suggest you stick to round limits.

[Kax-y]

@kostis
Do you have any idea why the RISE server mealy model fails its CI test? It seems to be the same from what I can see.

It might be the same reason as to why the RA one also fails its hypothesis comparison.

[kostis]

Do you have any idea why the RISE server mealy model fails its CI test? It seems to be the same from what I can see.

No idea. I find it strange, but it is happening all times, so it's clearly not a fluke. Something is wrong there. I'll try to check the changed files and see if something differs.

It might be the same reason as to why the RA one also fails its hypothesis comparison.

Yes, of course. It's perfectly normal that learning a RA fails if we cannot learn a Mealy machine one.

[actyp]

RISE Server

The problem that the RISE server exhibited was resolved in #133, it was hopefully just a timing issue. It is surprising that it appeared suddenly, but it seems that learning works now as expected. So rebase on top of main to see the new run.

CI Files

About the ci.yml changes:

I like the upload-artifact action, because we can download and inspect the results, in case something goes wrong. I don't know about the storage, but leave it there and we will see.

The SIFIS-HOME parts instead of deleting them all together, just include them commented out (as they were). Eventually, we will remove them if we cannot compile them, but for the time being leave them in the file.

[00oskpet]

@kostis This branch requires changes that are not in psf yet, I've made a pull-request for them here: protocol-fuzzing/protocol-state-fuzzer#123

It is just one file that is the issue.

[kostis]

@00oskpet: My attempt to change to PSF's main branch was initially to resolve a conflict and then to see if its recent changes managed to resolve the failure of RISE-Server test (in Mealy machine mode). It unfortunately did not. So that particular problem lies somewhere else.

Let me add my .02 here, from experience. I suggest the following two action points:

1. The PR tries to minimize the changes with the main branch (of EDHOC). There are perhaps good reasons to do code cleanups, or augment main's current functionality (e.g., by uploading artifacts of CI, saving results in directories whose names contain mealy somewhere, etc.), but these are orthogonal changes and can be submitted in separate independent PRs that can be reviewed and merged separately. This will allow us to find the culprit (for RISE-Server's failure) more easily, and also to review the rest of the changes in smaller chunks.
2. The PR is based on a stable version of PSF: if it really needs some changes to PSF's functionality, we discuss it there, we all agree on it, and we merge it first. If there is a way to achieve its functionality without changes to PSF, we do this first and then we possibly do these changes in a separate PR and merge them as an optimization.

[00oskpet]

@kostis

2. The PR is based on a stable version of PSF: if it really needs some changes to PSF's functionality, we discuss it there, we all agree on it, and we merge it first. If there is a way to achieve its functionality without changes to PSF, we do this first and then we possibly do these changes in a separate PR and merge them as an optimization.

What the pull-request in PSF addresses is the fact that DummyAlphabetBuilder had unimplemented methods that due to changes in PSF now are called, which causes them to raise exceptions. Due to the RA-changes not being merged and left unfixed(due to Linus being busy) this issue passed under the radar until we got the RA-branch ported and compiling on the latest version of PSF. This was necessary to use the EnumAlphabet for alphabet construction, since there is no option to pass an already created alphabet to PSF. If we want to avoid changing PSF we would need to create the alphabet in some other way.

[actyp]

I think since the AlphabetBuilderWrapper is already there in PSF, there is no reason not to merge the new (trivial) modifications.

Otherwise, if it was not there, then it would be possible to create here, in edhoc-fuzzer, the appropriate AlphabetBuilder, right?

[00oskpet]

Otherwise, if it was not there, then it would be possible to create here, in edhoc-fuzzer, the appropriate AlphabetBuilder, right?

It should be possible, yes. I Believe it might have been in edhoc-fuzzer originally long ago, but then moved to PSF in case someone else wanted to use it.

[kostis]

I've updated the main branch to use the latest PSF commit, which includes the changes to AlphabetBuilderEnum. You may want to rebase this branch on top of current main, adjust it appropriately, and possibly also follow my advice on trying to minimize the changes so that we can investigate, hopefully more easily, the failure(s) of RISE-Server and Lakers-Server.