Skip to content
New issue

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

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

Already on GitHub? # to your account

B 155 fix #89

Merged
merged 4 commits into from
Sep 20, 2022
Merged

B 155 fix #89

merged 4 commits into from
Sep 20, 2022

Conversation

baxterjfinch
Copy link
Contributor

No description provided.

@izelnakri izelnakri merged commit ddea01b into izelnakri:main Sep 20, 2022
@izelnakri
Copy link
Owner

Hi @baxterjfinch , have you run the tests? Our CI was broken so I merged the PR, however I just realized locally tests are failing, I cant cut a release without fixing the test suite. Please create another PR with test suite fixes, then we can cut a release. You can run the test suite by: $ mix test --seed 0 Thanks.

@baxterjfinch
Copy link
Contributor Author

Hello @izelnakri , thanks! I'll take a look at the tests today. I'm still not sure that the v value is being constructed correctly. Have you been able to send transactions that are EIP-155 compliant?

A transaction we have used to test still comes back with a v value of 28:
https://www.ethereumdecoder.com/?search=0xb1b0a25362b65e9fef96fb09c9cbffc9993a0ed5ab66adbbd2895c632d67c402

We use Alchemy as our provider and they are not going to allow non-compliant EIP-155 transactions soon so we're hoping to get a fix into this library soon. Thanks! :)

@albertoramires
Copy link

albertoramires commented Feb 9, 2023

@izelnakri @baxterjfinch sorry to bother you guys, but is this working for anything other than 0-9 chain ids? If I use Optimism Goerli (420), <<chain_id_int>> = chain_id won't match. 🤔

@izelnakri
Copy link
Owner

izelnakri commented Feb 10, 2023

@albertoramires ETH provides no guarantee or promises at this point, we dont even have a working test suite now for a while. My time needs to be managed properly. Please read the code and contribute to project if it isn't serving your needs at this point. Thanks!

@albertoramires
Copy link

@izelnakri no worries, was making sure it wasn't just an issue on my side. Thank you.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants