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

feat: Lotus Send CLI: Lotus send should work with ETH address receipients #12319

Merged
merged 5 commits into from
Jul 30, 2024
Merged
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -14,6 +14,7 @@
- https://github.com/filecoin-project/lotus/pull/12279 Upgrade to go-f3 v0.0.5
- https://github.com/filecoin-project/lotus/pull/12295 Upgrade to go-f3 v0.0.6
- https://github.com/filecoin-project/lotus/pull/12292: feat: p2p: allow overriding bootstrap nodes with environmemnt variable
- https://github.com/filecoin-project/lotus/pull/12319: feat: `lotus send CLI`: allow sending to ETH addresses

## New features

25 changes: 23 additions & 2 deletions cli/send.go
Original file line number Diff line number Diff line change
@@ -90,7 +90,20 @@ var SendCmd = &cli.Command{

params.To, err = address.NewFromString(cctx.Args().Get(0))
if err != nil {
return ShowHelp(cctx, fmt.Errorf("failed to parse target address: %w", err))
// could be an ETH address
ea, err := ethtypes.ParseEthAddress(cctx.Args().Get(0))
if err != nil {
return ShowHelp(cctx, fmt.Errorf("failed to parse target address; address must be a valid FIL address or an ETH address: %w", err))
}
// this will be either "f410f..." or "f0..."
params.To, err = ea.ToFilecoinAddress()
if err != nil {
return ShowHelp(cctx, fmt.Errorf("failed to convert ETH address to FIL address: %w", err))
}
// ideally, this should never happen
if !(params.To.Protocol() == address.ID || params.To.Protocol() == address.Delegated) {
return ShowHelp(cctx, fmt.Errorf("ETH addresses can only map to a FIL addresses starting with f410f or f0"))
}
}

val, err := types.ParseFIL(cctx.Args().Get(1))
@@ -118,6 +131,12 @@ var SendCmd = &cli.Command{
}
fmt.Println("f4 addr: ", faddr)
params.From = faddr
} else {
defaddr, err := srv.FullNodeAPI().WalletDefaultAddress(ctx)
if err != nil {
return fmt.Errorf("failed to get default address: %w", err)
}
params.From = defaddr
}

if cctx.IsSet("params-hex") {
@@ -128,7 +147,7 @@ var SendCmd = &cli.Command{
params.Params = decparams
}

if ethtypes.IsEthAddress(params.From) {
if ethtypes.IsEthAddress(params.From) || ethtypes.IsEthAddress(params.To) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this is going to cause some issues.

  1. There are almost certainly users using native accounts to send messages to EVM smart contracts, explicitly specifying the method number.
  2. Implicitly calling InvokeContract is the nice thing to do, but that's a significant change of behavior.

Ideally this would have this method number handling if and only if the user passes a literal 0x... address in the to field. If they pass a normal Filecoin address, they should get the normal Filecoin method handling.

Copy link
Member

Choose a reason for hiding this comment

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

pulled this up into a new issue #12326

// Method numbers don't make sense from eth accounts.
if cctx.IsSet("method") {
return xerrors.Errorf("messages from f410f addresses may not specify a method number")
@@ -167,6 +186,8 @@ var SendCmd = &cli.Command{
params.Method = abi.MethodNum(cctx.Uint64("method"))
}

_, _ = fmt.Fprintf(cctx.App.Writer, "Sending message from: %s\nSending message to: %s\nUsing Method: %d\n", params.From.String(), params.To.String(), params.Method)
rvagg marked this conversation as resolved.
Show resolved Hide resolved

if cctx.IsSet("gas-premium") {
gp, err := types.BigFromString(cctx.String("gas-premium"))
if err != nil {
11 changes: 6 additions & 5 deletions cli/send_test.go
Original file line number Diff line number Diff line change
@@ -59,16 +59,17 @@ func TestSendCLI(t *testing.T) {

gomock.InOrder(
mockSrvcs.EXPECT().MessageForSend(gomock.Any(), SendParams{
To: mustAddr(address.NewIDAddress(1)),
Val: oneFil,
From: mustAddr(address.NewIDAddress(1)),
To: mustAddr(address.NewIDAddress(1)),
Val: oneFil,
}).Return(arbtProto, nil),
mockSrvcs.EXPECT().PublishMessage(gomock.Any(), arbtProto, false).
Return(sigMsg, nil, nil),
mockSrvcs.EXPECT().Close(),
)
err := app.Run([]string{"lotus", "send", "t01", "1"})
err := app.Run([]string{"lotus", "send", "--from", "t01", "t01", "1"})
require.NoError(t, err)
require.EqualValues(t, sigMsg.Cid().String()+"\n", buf.String())
require.Contains(t, buf.String(), sigMsg.Cid().String()+"\n")
})
}

@@ -112,6 +113,6 @@ func TestSendEthereum(t *testing.T) {
)
err = app.Run([]string{"lotus", "send", "--from-eth-addr", testEthAddr.String(), "--params-hex", "01020304", "f01", "1"})
require.NoError(t, err)
require.EqualValues(t, sigMsg.Cid().String()+"\n", buf.String())
require.Contains(t, buf.String(), sigMsg.Cid().String()+"\n")
})
}