Skip to content
This repository has been archived by the owner on May 22, 2023. It is now read-only.

Use EIP-155 signing for Ethereum contracts #725

Merged
merged 6 commits into from
Mar 11, 2021

Conversation

Shadowfiend
Copy link
Contributor

go-ethereum 1.10.0 has switched to denying non-EIP155 signatures by default.
Although our code uses go-ethereum's own ABI binding generation, the version
currently in our dependencies never added support for EIP155 signers, and
instead always uses Homestead signers for the transactors it provides helpers
for.

This commit bumps the keep-common version to a hotfix that enables EIP-155
signing for contract bindings. It updates generated code from keep-common to
reflect these new contract bindings, and updates Ethereum client instantiation
to resolve the chain id and pass it into the contract binding constructor
functions.

go-ethereum 1.10.0 has switched to denying non-EIP155 signatures by default.
Although our code uses go-ethereum's own ABI binding generation, the version
currently in our dependencies never added support for EIP155 signers, and
instead always uses Homestead signers for the transactors it provides helpers
for.

This commit bumps the keep-common version to a hotfix that enables EIP-155
signing for contract bindings. It updates generated code from keep-common to
reflect these new contract bindings, and updates Ethereum client instantiation
to resolve the chain id and pass it into the contract binding constructor
functions.
go.sum Outdated
@@ -1,3 +1,4 @@
<<<<<<< Updated upstream
Copy link
Member

Choose a reason for hiding this comment

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

Merge conflict?

Copy link
Member

Choose a reason for hiding this comment

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

Dropped in 1baa1eb

chainID is used to create a custom bin.TransactOpts with EIP-155 signer
in each New* function of generated contract. Storing chainID in
EthereumChain struct is required so that the value can be passed to
NewBondedECDSAKeep in ethereum.go. Bonded ECDSA Keep Go contract binding
is created on-the-fly.
It is the same version as the one referenced before except that it is
tagged.
The regenerated TBTC Deposit contract binding (see tbtc dependency
version update in the previous commit), has EIP155-compatible signer
requiring to pass a chain ID during construction.
@pdyraga pdyraga added this to the v1.7.1 milestone Mar 11, 2021
@pdyraga pdyraga marked this pull request as ready for review March 11, 2021 12:07
@pdyraga
Copy link
Member

pdyraga commented Mar 11, 2021

Tested:

  • client without a fix can not submit a TX to geth v1.10.1 with default start options
  • client with a fix can submit a TX to geth v1.10.1 with default start options
  • client with a fix can submit a TX to geth v1.10.1 with --rpc.allow-unprotected-txs
  • client with a fix can submit a TX to (old) geth v1.9.18

Copy link
Member

@pdyraga pdyraga left a comment

Choose a reason for hiding this comment

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

LGTM but given that I was committing here, we need another approval.

@dimpar dimpar merged commit 56b4f55 into releases/mainnet/v1.7.0 Mar 11, 2021
@dimpar dimpar deleted the keep-ecdsa-hotfix branch March 11, 2021 13:31
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.

4 participants