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.

Sign up for GitHub

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

Jump to bottom

Refactor: Unify EthTx to FilecoinMessage methods v2 #10095

Merged
merged 7 commits into from
Jan 30, 2023
Merged

Refactor: Unify EthTx to FilecoinMessage methods v2 #10095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
edad5f2
Refactor: Unify EthTx to FilecoinMessage methods
arajasek Jan 14, 2023
a496300
Filecoin messages can again be converted to Eth Txs
Jan 16, 2023
dd5f8b0
All BLS messages should calculated tx hash with unsigned message
Jan 23, 2023
5b3e4ee
Refactor newEthTxReceipt
Jan 23, 2023
93bab0e
fill in from and to for non-eth transactions
Jan 23, 2023
7814d72
Hoist nil check out of newEthTxFromMessageLookup
Jan 27, 2023
f15310a
Merge branch 'release/v1.20.0' into gstuart/unify-tx-hash
raulk Jan 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion chain/messagesigner/messagesigner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ func (ms *MessageSigner) dstoreKey(addr address.Address) datastore.Key {

func SigningBytes(msg *types.Message, sigType crypto.SigType) ([]byte, error) {
if sigType == crypto.SigTypeDelegated {
txArgs, err := ethtypes.EthTxArgsFromMessage(msg)
txArgs, err := ethtypes.EthTxArgsFromUnsignedEthMessage(msg)
if err != nil {
return nil, xerrors.Errorf("failed to reconstruct eth transaction: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion chain/signatures.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func AuthenticateMessage(msg *types.SignedMessage, signer address.Address) error
typ := msg.Signature.Type
switch typ {
case crypto.SigTypeDelegated:
txArgs, err := ethtypes.EthTxArgsFromMessage(&msg.Message)
txArgs, err := ethtypes.EthTxArgsFromUnsignedEthMessage(&msg.Message)
if err != nil {
return xerrors.Errorf("failed to reconstruct eth transaction: %w", err)
}
Expand Down
39 changes: 38 additions & 1 deletion chain/types/ethtypes/eth_transactions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,44 @@ type EthTxArgs struct {
S big.Int `json:"s"`
}

func EthTxArgsFromMessage(msg *types.Message) (EthTxArgs, error) {
// EthTxFromSignedEthMessage does NOT populate:
// - BlockHash
// - BlockNumber
// - TransactionIndex
// - From
// - Hash
func EthTxFromSignedEthMessage(smsg *types.SignedMessage) (EthTx, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Eth{...}Message is a confusing name as it leads to thinking that this is an Eth message/tx, but it's actually a Filecoin one.

Suggested change
func EthTxFromSignedEthMessage(smsg *types.SignedMessage) (EthTx, error) {
func EthTxFromSignedMessage(smsg *types.SignedMessage) (EthTx, error) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We kind of need some way of differentiating between types.Message/types.SignedMessage that can be converted into a legitimate EthTx or not. It makes sense to me to use Transaction/Tx for the Ethereum shape, and Message for the Filecoin shape, but I know other people might have differing thoughts about those words.

I just want to avoid the confusion of someone thinking they can use this function for any signed message.

if smsg.Signature.Type != typescrypto.SigTypeDelegated {
return EthTx{}, xerrors.Errorf("signature is not delegated type, is type: %d", smsg.Signature.Type)
}

txArgs, err := EthTxArgsFromUnsignedEthMessage(&smsg.Message)
if err != nil {
return EthTx{}, xerrors.Errorf("failed to convert the unsigned message: %w", err)
}

r, s, v, err := RecoverSignature(smsg.Signature)
if err != nil {
return EthTx{}, xerrors.Errorf("failed to recover signature: %w", err)
}

return EthTx{
Nonce: EthUint64(txArgs.Nonce),
ChainID: EthUint64(txArgs.ChainID),
To: txArgs.To,
Value: EthBigInt(txArgs.Value),
Type: Eip1559TxType,
Gas: EthUint64(txArgs.GasLimit),
MaxFeePerGas: EthBigInt(txArgs.MaxFeePerGas),
MaxPriorityFeePerGas: EthBigInt(txArgs.MaxPriorityFeePerGas),
V: v,
R: r,
S: s,
Input: txArgs.Input,
}, nil
}

func EthTxArgsFromUnsignedEthMessage(msg *types.Message) (EthTxArgs, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
func EthTxArgsFromUnsignedEthMessage(msg *types.Message) (EthTxArgs, error) {
func EthTxArgsFromUnsignedMessage(msg *types.Message) (EthTxArgs, error) {

var (
to *EthAddress
params []byte
Expand Down
8 changes: 4 additions & 4 deletions itests/eth_account_abstraction_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestEthAccountAbstraction(t *testing.T) {
msgFromPlaceholder, err = client.GasEstimateMessageGas(ctx, msgFromPlaceholder, nil, types.EmptyTSK)
require.NoError(t, err)

txArgs, err := ethtypes.EthTxArgsFromMessage(msgFromPlaceholder)
txArgs, err := ethtypes.EthTxArgsFromUnsignedEthMessage(msgFromPlaceholder)
require.NoError(t, err)

digest, err := txArgs.ToRlpUnsignedMsg()
Expand Down Expand Up @@ -108,7 +108,7 @@ func TestEthAccountAbstraction(t *testing.T) {
msgFromPlaceholder, err = client.GasEstimateMessageGas(ctx, msgFromPlaceholder, nil, types.EmptyTSK)
require.NoError(t, err)

txArgs, err = ethtypes.EthTxArgsFromMessage(msgFromPlaceholder)
txArgs, err = ethtypes.EthTxArgsFromUnsignedEthMessage(msgFromPlaceholder)
require.NoError(t, err)

digest, err = txArgs.ToRlpUnsignedMsg()
Expand Down Expand Up @@ -180,7 +180,7 @@ func TestEthAccountAbstractionFailure(t *testing.T) {
require.NoError(t, err)

msgFromPlaceholder.Value = abi.TokenAmount(types.MustParseFIL("1000"))
txArgs, err := ethtypes.EthTxArgsFromMessage(msgFromPlaceholder)
txArgs, err := ethtypes.EthTxArgsFromUnsignedEthMessage(msgFromPlaceholder)
require.NoError(t, err)

digest, err := txArgs.ToRlpUnsignedMsg()
Expand Down Expand Up @@ -218,7 +218,7 @@ func TestEthAccountAbstractionFailure(t *testing.T) {
msgFromPlaceholder, err = client.GasEstimateMessageGas(ctx, msgFromPlaceholder, nil, types.EmptyTSK)
require.NoError(t, err)

txArgs, err = ethtypes.EthTxArgsFromMessage(msgFromPlaceholder)
txArgs, err = ethtypes.EthTxArgsFromUnsignedEthMessage(msgFromPlaceholder)
require.NoError(t, err)

digest, err = txArgs.ToRlpUnsignedMsg()
Expand Down
Loading