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

seth: non checksum addresses supported in ETH_FROM #796

Merged
merged 2 commits into from
Sep 18, 2021
Merged

Conversation

d-xo
Copy link
Contributor

@d-xo d-xo commented Sep 15, 2021

Description

Fixes an issue in #771, where if ETH_RPC_ACCOUNTS was set, and ETH_FROM was not a checksummed address, the signing request would be incorrectly forwarded to the rpc node, even if ETH_FROM was present in a local keystore.

The issue is fixed by using a case insensitive grep when checking if ETH_FROM is present in a local keystore.

Checklist

  • tested locally
  • added automated tests
  • updated the docs
  • updated the changelog

@d-xo d-xo force-pushed the fix-checksum-lookup branch 2 times, most recently from 60f8f50 to da7f49e Compare September 15, 2021 11:19
Fixes an issue in #771, where if `ETH_RPC_ACCOUNTS` was set, and
`ETH_FROM` was not a checksummed address, the signing request would be
incorrectly forwarded to the rpc node, even if `ETH_FROM` was present in
a local keystore.

The issue is fixed by using a case insensitive grep when checking if
`ETH_FROM` is present in a local keystore.
@d-xo d-xo linked an issue Sep 15, 2021 that may be closed by this pull request
src/seth/CHANGELOG.md Outdated Show resolved Hide resolved
@@ -49,10 +49,52 @@ dapp_testnet() {

# dynamic fee transaction (EIP-1559)
seth send "$A_ADDR" "on()" --gas 0xffff --password /dev/null --from "$ACC" --keystore "$TMPDIR"/8545/keystore --prio-fee 2gwei --gas-price 10gwei

# clean up
killall geth
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Somehow this broke mktemp in the subsequent tests. Not sure why. I redid the dapp testnet stuff in #797 anyway.


dapp testnet --dir "$TMPDIR" &
# give it a few secs to start up
sleep 180
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, you can make this like 3-10 seconds and it still works well enough. Should make CI faster

@d-xo d-xo merged commit 084c96b into master Sep 18, 2021
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.

Checksum address when reading from ETH_FROM
3 participants