Fix no trusted certificates

[fasmat]

While fixing failing tests in spacemeshos/go-spacemesh#6313 I noticed this issue.

If filepath.Walk in LoadTrustedPublicKeys fails r.trustedCertifierKeys will not be updated. This means that if this happens during startup no certifier keys will be trusted, not even the one defined in the config.

This updates the code in a way such that if loading keys from disk fails at least the certifier key from the config will still be valid to be used for submissions.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.6%. Comparing base (6d0d2d5) to head (1843441).

Additional details and impacted files

@@           Coverage Diff           @@
##           develop    #523   +/-   ##
=======================================
  Coverage     80.6%   80.6%           
=======================================
  Files           27      27           
  Lines         1868    1869    +1     
=======================================
+ Hits          1506    1507    +1     
  Misses         231     231           
  Partials       131     131           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[poszu]

If it happens at startup, then poet will shutdown with an error. If it happens on a GRPC, I think the whole call should be ineffective - the keys should remain as they were before the call. That's why the r.trustedCertifierKeys is updated only if everything went well, at the very end of LoadTrustedPublicKeys().

[fasmat]

If it happens at startup, then poet will shutdown with an error. If it happens on a GRPC, I think the whole call should be ineffective - the keys should remain as they were before the call. That's why the r.trustedCertifierKeys is updated only if everything went well, at the very end of LoadTrustedPublicKeys().

I'm not sure this is true, e2e tests in spacemeshos/go-spacemesh#6313 got stuck when verifying certificates and just kept re-trying to submit challenges to a poet that would always respond with certificate invalid (because it had 0 trusted certificates in the r.trustedKeys map). The fix in the test was the addition of this little helper: https://github.com/spacemeshos/go-spacemesh/blob/poet-issue-501/activation/e2e/poet_test.go#L90-L94 and ensuring that the passed directory path points to a directory that actually exists. The change in this PR just makes sure that if (re-)loading certificates fails certificates created from the certifier defined in the config can still be verified.

I still think this should be fixed, or failing to reload certificates will disable PoET to validate any submission even the ones using the certifier that is set in the config.

[poszu]

If it happens at startup, then poet will shutdown with an error. If it happens on a GRPC, I think the whole call should be ineffective - the keys should remain as they were before the call. That's why the r.trustedCertifierKeys is updated only if everything went well, at the very end of LoadTrustedPublicKeys().

I'm not sure this is true, e2e tests in spacemeshos/go-spacemesh#6313 got stuck when verifying certificates and just kept re-trying to submit challenges to a poet that would always respond with certificate invalid (because it had 0 trusted certificates in the r.trustedKeys map). The fix in the test was the addition of this little helper: spacemeshos/go-spacemesh@poet-issue-501/activation/e2e/poet_test.go#L90-L94 and ensuring that the passed directory path points to a directory that actually exists. The change in this PR just makes sure that if (re-)loading certificates fails certificates created from the certifier defined in the config can still be verified.

Poet should shutdown because an error is bubbled up here:

poet/registration/registration.go

Lines 165 to 167 in ae0594a

	if err := r.LoadTrustedPublicKeys(ctx); err != nil {
	return nil, fmt.Errorf("loading trusted public keys: %w", err)
	}

I still think this should be fixed, or failing to reload certificates will disable PoET to validate any submission even the ones using the certifier that is set in the config.

That's not the case. If reloading certificates from a directory goes wrong, the existing list of keys remains untouched. This change actually makes the situation worse by removing all keys but the main one.

[fasmat]

But in the scenario I'm talking about TrustedKeysDirPath is "" so the code you shared is never executed, yet every request still fails to verify.

EDIT: and that means my fix isn't complete