Eth JSON-RPC: use ToFilecoinAddress() to get f4 addr and validate v sig + cleanups #9970
Conversation
ychiaoli18
changed the title
| // EIP-1559 and EIP-2930 transactions only support 0 or 1 for v | ||
| // Legacy and EIP-155 transactions support other values | ||
| // https://github.com/ethers-io/ethers.js/blob/56fabe987bb8c1e4891fdf1e5d3fe8a4c0471751/packages/transactions/src.ts/index.ts#L333 | ||
| if !v.Equals(big.NewInt(0)) && !v.Equals(big.NewInt(1)) { | ||
| return nil, fmt.Errorf("EIP-1559 transactions only support 0 or 1 for v") | ||
| } |
There was a problem hiding this comment.
Would like to add an integration test. Recorded in #9849.
| } | ||
|
|
||
| func NewEthAddressFromFilecoinAddress(addr address.Address) (EthAddress, error) { | ||
| ethAddr, ok, err := TryEthAddressFromFilecoinAddress(addr, true) |
There was a problem hiding this comment.
I know this is not touched here, but the interface of this function is very tricky, and that trickiness leaks here. It only returns ok=false, err=nil in one case: when we are not allowing IDs and the supplied address is an ID address. I would simplify this to return the EthAddress and an error only, with a predefined error when the supplied address is an ID address.
If we do that, we can get rid of NewEthAddressFromFilecoinAddress and use TryEthAddressFromFilecoinAddress directly everywhere, which is also a better name because this is really a conversion, not a construction.
lib/sigs/delegated/eth.go
Outdated
| package delegated | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "golang.org/x/crypto/sha3" | ||
| ) | ||
|
|
||
| // EthAddressFromPubKey returns the Ethereum address corresponding to an | ||
| // uncompressed secp256k1 public key. | ||
| // | ||
| // TODO move somewhere else, this likely doesn't belong here. | ||
| func EthAddressFromPubKey(pubk []byte) ([]byte, error) { | ||
| // if we get an uncompressed public key (that's what we get from the library, | ||
| // but putting this check here for defensiveness), strip the prefix | ||
| if pubk[0] != 0x04 { | ||
| return nil, fmt.Errorf("expected first byte of secp256k1 to be 0x04 (uncompressed)") | ||
| } | ||
| pubk = pubk[1:] | ||
|
|
||
| // Calculate the Ethereum address based on the keccak hash of the pubkey. | ||
| hasher := sha3.NewLegacyKeccak256() | ||
| hasher.Write(pubk) | ||
| ethAddr := hasher.Sum(nil)[12:] | ||
| return ethAddr, nil | ||
| } |
There was a problem hiding this comment.
This was already covered in #9984, which landed in the branch. Will resolve the conflict.
raulk
changed the title
raulk
force-pushed
the
fix/delegated-addr
branch
from
d3fd493 to
81a7abd
Compare
Related Issues
Closes filecoin-project/ref-fvm#1030.
Closes filecoin-project/ref-fvm#1335.
Proposed Changes
Additional Info
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>fix: mempool: Introduce a cache for valid signaturesPR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, testarea, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps