hashgraph / hedera-smart-contracts

Contains Hedera Smart Contract Service supporting files
Apache License 2.0
37 stars 50 forks source link

test: ecrecover evm compatibility #815

Open marcin-piela-ariane opened 1 week ago

marcin-piela-ariane commented 1 week ago

Description:

This PR adds tests and documents the behavior of the ecrecover function with various keys, providing developers with clear examples of when ecrecover works correctly.

Related issue(s):

Closes #814

Notes for reviewer:

Checklist

marcin-piela-ariane commented 1 week ago

Great work but please include artifact files.

@quiet-node Sorry for that. Added ABI & adjusted to the new directory structure.

marcin-piela-ariane commented 1 week ago

Looks like you're targeting the merge against hashgraph:main branch, instead of main. Is this on purpose?

@quiet-node Yes, I'd like to contribute to the main project from my forked repository (branch test-ecrecover-evm-compatibility).

quiet-node commented 1 week ago

Looks like you're targeting the merge against hashgraph:main branch, instead of main. Is this on purpose?

@quiet-node Yes, I'd like to contribute to the main project from my forked repository (branch test-ecrecover-evm-compatibility).

Oh ah yeah sorry I completely forgot they were technically the same branch. 😅

marcin-piela-ariane commented 1 week ago

@quiet-node

I tried running the test and none of them were working...

Did you set OPERATOR_ID_A & PRIVATE_KEYS as ECDSA keys?

Important: While Hedera supports both ECDSA and ED25519 accounts, please use ECDSA since Ethereum only supports ECDSA. (source: TEST_SETUP.md)

quiet-node commented 5 days ago

@quiet-node

I tried running the test and none of them were working...

Did you set OPERATOR_ID_A & PRIVATE_KEYS as ECDSA keys?

Important: While Hedera supports both ECDSA and ED25519 accounts, please use ECDSA since Ethereum only supports ECDSA. (source: TEST_SETUP.md)

Ah yeah I'm aware of those options and I have been using EDCSA. I just switched to different environment, testnet, and it works fine. However, none of them were passing on localnet.

marcin-piela-ariane commented 5 days ago

@quiet-node

I tried running the test and none of them were working...

Did you set OPERATOR_ID_A & PRIVATE_KEYS as ECDSA keys?

Important: While Hedera supports both ECDSA and ED25519 accounts, please use ECDSA since Ethereum only supports ECDSA. (source: TEST_SETUP.md)

Ah yeah I'm aware of those options and I have been using EDCSA. I just switched to different environment, testnet, and it works fine. However, none of them were passing on localnet.

@quiet-node OK. Thanks. I have to admit I didn't test it on local node. Only on testnet. Will have to dig into it.

marcin-piela-ariane commented 4 days ago

@quiet-node Fixed for local node testing. Wrong mirror node URL was the problem. Also added a retry mechanism for mirror node querying. Should be good to go now.