Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: lib/runtime: update sign_ funcs to return fixed size byte optional #1451

Merged
merged 2 commits into from
Mar 12, 2021

Conversation

noot
Copy link
Contributor

@noot noot commented Mar 11, 2021

Changes

  • update sign_ functs to return fixed size byte optional
  • update life not to error if core_version doesn't exist (needed for host API tester)

Tests

go test ./lib/runtime/wasmer/ -run Test_ext_crypto_ed25519_sign_version_1
go test ./lib/runtime/wasmer/ -run Test_ext_crypto_sr25519_sign_version_1
go test ./lib/runtime/life

Checklist

  • I have read CODE_OF_CONDUCT and CONTRIBUTING
  • I have provided as much information as possible and necessary
  • I have reviewed my own pull request before requesting a review
  • All integration tests and required coverage checks are passing

Issues

related to #1105

@noot noot changed the title fix: lib/runtime: update sign_ functs to return fixed size byte optional fix: lib/runtime: update sign_ funcs to return fixed size byte optional Mar 11, 2021
@noot noot self-assigned this Mar 11, 2021
@FlorianFranzen
Copy link
Contributor

FlorianFranzen commented Mar 11, 2021

This does indeed fix the encoding issue and the returned signature now pass verification.

I would however like to ask you to increase the error level on verification failures in ed25519:

--- a/lib/runtime/wasmer/imports.go
+++ b/lib/runtime/wasmer/imports.go
@@ -636,7 +636,7 @@ func ext_crypto_sr25519_verify_version_1(context unsafe.Pointer, sig C.int32_t,
   }

   if ok, err := pub.VerifyDeprecated(message, signature); err != nil || !ok {
-    logger.Debug("[ext_crypto_sr25519_verify_version_1] failed to verify sr25519 signature")
+    logger.Error("[ext_crypto_sr25519_verify_version_1] failed to verify sr25519 signature")
     // TODO: fix this, fails at block 3876
     return 1
   }

The life issue however still exists, as the call to a none-existing Version() still causes a panic. Maybe calling Version() similar to how it is done on the wasmer backend (e.g. when the version field is needed but nil and not on initialization) would be the easiest solution.

@noot noot merged commit 62da970 into development Mar 12, 2021
@noot noot deleted the noot/fix-exts branch March 12, 2021 01:02
noot added a commit that referenced this pull request Mar 12, 2021
github-actions bot pushed a commit that referenced this pull request Mar 12, 2021
noot: update sign_ functs to return fixed size byte optional (#1451)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants