Skip to content
This repository has been archived by the owner on Apr 15, 2024. It is now read-only.

test: add EVM signature methods tests #103

Merged
merged 5 commits into from
Feb 9, 2023

Conversation

rach-id
Copy link
Member

@rach-id rach-id commented Feb 7, 2023

Overview

Closes: #53

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@rach-id rach-id added the testing label Feb 7, 2023
@rach-id rach-id requested a review from rahulghangas February 7, 2023 16:32
@rach-id rach-id self-assigned this Feb 7, 2023
@rach-id rach-id requested a review from evan-forbes as a code owner February 7, 2023 16:32
@rach-id rach-id added the evm label Feb 7, 2023
Copy link
Contributor

@rahulghangas rahulghangas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff, one question (unrelated to PR)


func TestSigToVRS(t *testing.T) {
sig := "0xca2aa01f5b32722238e8f45356878e2cfbdc7c3335fbbf4e1dc3dfc53465e3e137103769d6956414014ae340cc4cb97384b2980eea47942f135931865471031a00"
v, r, s := evm.SigToVRS(sig)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SigToVRS can potentially give invalid values. Since an invalid signature can be passed in, maybe we need to refactor this function to return an error

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It expects the signature to be valid as a precondition, But we could make it better, created issue: #105

evm/ethereum_signature_test.go Outdated Show resolved Hide resolved
Co-authored-by: Rahul Ghangas <rahulghangas0@gmail.com>
@rach-id rach-id requested a review from rahulghangas February 9, 2023 09:36
@rach-id rach-id merged commit 7162094 into celestiaorg:main Feb 9, 2023
@rach-id rach-id deleted the evm_sign_test branch February 9, 2023 11:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests for EVM signing methods
2 participants