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

Improve Eth JSON-RPC API handling of native messages #11355

Closed
Stebalien opened this issue Oct 26, 2023 · 1 comment · Fixed by #11609
Closed

Improve Eth JSON-RPC API handling of native messages #11355

Stebalien opened this issue Oct 26, 2023 · 1 comment · Fixed by #11609
Assignees

Comments

@Stebalien
Copy link
Member

E.g., code like

func ethTxFromNativeMessage(ctx context.Context, msg *types.Message, sa StateAPI) ethtypes.EthTx {
// We don't care if we error here, conversion is best effort for non-eth transactions
from, _ := lookupEthAddress(ctx, msg.From, sa)
to, _ := lookupEthAddress(ctx, msg.To, sa)
return ethtypes.EthTx{
To: &to,
From: from,
Nonce: ethtypes.EthUint64(msg.Nonce),
ChainID: ethtypes.EthUint64(build.Eip155ChainId),
Value: ethtypes.EthBigInt(msg.Value),
Type: ethtypes.Eip1559TxType,
Gas: ethtypes.EthUint64(msg.GasLimit),
MaxFeePerGas: ethtypes.EthBigInt(msg.GasFeeCap),
MaxPriorityFeePerGas: ethtypes.EthBigInt(msg.GasPremium),
AccessList: []ethtypes.EthHash{},
}
}

Right now, we:

  1. Blindly convert without translating parameters, etc.
  2. Discard, e.g., the message number.
  3. Ignore errors converting from/to addresses.

Instead, we need to:

  1. Translate parameters & return values the same way we translate them in the tracing API. I.e., we need to call these two methods:
    func handleFilecoinMethodInput(method abi.MethodNum, codec uint64, params []byte) ([]byte, error) {
    NATIVE_METHOD_SELECTOR := []byte{0x86, 0x8e, 0x10, 0xc4}
    EVM_WORD_SIZE := 32
    staticArgs := []uint64{
    uint64(method),
    codec,
    uint64(EVM_WORD_SIZE) * 3,
    uint64(len(params)),
    }
    totalWords := len(staticArgs) + (len(params) / EVM_WORD_SIZE)
    if len(params)%EVM_WORD_SIZE != 0 {
    totalWords++
    }
    len := 4 + totalWords*EVM_WORD_SIZE
    w := &bytes.Buffer{}
    err := binary.Write(w, binary.BigEndian, NATIVE_METHOD_SELECTOR)
    if err != nil {
    return nil, fmt.Errorf("handleFilecoinMethodInput: failed writing method selector: %w", err)
    }
    for _, arg := range staticArgs {
    err := writePadded(w, arg, 32)
    if err != nil {
    return nil, fmt.Errorf("handleFilecoinMethodInput: %w", err)
    }
    }
    err = binary.Write(w, binary.BigEndian, params)
    if err != nil {
    return nil, fmt.Errorf("handleFilecoinMethodInput: failed writing params: %w", err)
    }
    remain := len - w.Len()
    for i := 0; i < remain; i++ {
    err = binary.Write(w, binary.BigEndian, uint8(0))
    if err != nil {
    return nil, fmt.Errorf("handleFilecoinMethodInput: failed writing tailing zeros: %w", err)
    }
    }
    return w.Bytes(), nil
    }
    func handleFilecoinMethodOutput(exitCode exitcode.ExitCode, codec uint64, data []byte) ([]byte, error) {
    w := &bytes.Buffer{}
    values := []interface{}{uint32(exitCode), codec, uint32(w.Len()), uint32(len(data))}
    for _, v := range values {
    err := writePadded(w, v, 32)
    if err != nil {
    return nil, fmt.Errorf("handleFilecoinMethodOutput: %w", err)
    }
    }
    err := binary.Write(w, binary.BigEndian, data)
    if err != nil {
    return nil, fmt.Errorf("handleFilecoinMethodOutput: failed writing data: %w", err)
    }
    return w.Bytes(), nil
    }
  2. Try harder to lookup the correct addresses. Although there will likely be some impossible cases.
@Stebalien
Copy link
Member Author

Ignore errors converting from/to addresses.

So, an additional issue here is that we lookup addresses with an "empty tsk". That means we'll use whatever we consider to be our latest "head" instead of the state-tree after the message executed, and will fail (intermittently) on re-org.

Stebalien added a commit that referenced this issue Nov 7, 2023
We need to always use the state-tree from the tipset _after_ the message
executed. If we use any other state-tree, we might not find the address
we're trying to resolve.

This change also has some implication for pending messages: there's no
guarantee we'll be able to generate a 0x-style address for a pending
native message. So, instead of trying, I've removed support for pending
native messages from the Eth API. Messages from EthAccounts will still
work, and native messages will still show up in blocks/traces, they just
won't show up as "pending". Which should affect exactly nobody.

I'm also taking this opportunity to cleanup some edge-cases:

1. Pass contexts where appropriate.
2. Remove all state access from `ethTxHashFromSignedMessage`.

Part of #11355
Stebalien added a commit that referenced this issue Nov 7, 2023
We need to always use the state-tree from the tipset _after_ the message
executed. If we use any other state-tree, we might not find the address
we're trying to resolve.

This change also has some implication for pending messages: there's no
guarantee we'll be able to generate a 0x-style address for a pending
native message. So, instead of trying, I've removed support for pending
native messages from the Eth API. Messages from EthAccounts will still
work, and native messages will still show up in blocks/traces, they just
won't show up as "pending". Which should affect exactly nobody.

I'm also taking this opportunity to cleanup some edge-cases:

1. Pass contexts where appropriate.
2. Remove all state access from `ethTxHashFromSignedMessage`.

Part of #11355
Stebalien added a commit that referenced this issue Nov 17, 2023
We need to always use the state-tree from the tipset _after_ the message
executed. If we use any other state-tree, we might not find the address
we're trying to resolve.

This change also has some implication for pending messages: there's no
guarantee we'll be able to generate a 0x-style address for a pending
native message. So, instead of trying, I've removed support for pending
native messages from the Eth API. Messages from EthAccounts will still
work, and native messages will still show up in blocks/traces, they just
won't show up as "pending". Which should affect exactly nobody.

I'm also taking this opportunity to cleanup some edge-cases:

1. Pass contexts where appropriate.
2. Remove all state access from `ethTxHashFromSignedMessage`.

Part of #11355
Stebalien added a commit that referenced this issue Nov 17, 2023
We need to always use the state-tree from the tipset _after_ the message
executed. If we use any other state-tree, we might not find the address
we're trying to resolve.

This change also has some implication for pending messages: there's no
guarantee we'll be able to generate a 0x-style address for a pending
native message. So, instead of trying, I've removed support for pending
native messages from the Eth API. Messages from EthAccounts will still
work, and native messages will still show up in blocks/traces, they just
won't show up as "pending". Which should affect exactly nobody.

I'm also taking this opportunity to cleanup some edge-cases:

1. Pass contexts where appropriate.
2. Remove all state access from `ethTxHashFromSignedMessage`.

Part of #11355
Stebalien added a commit that referenced this issue Nov 17, 2023
When translating "native" messages to Ethereum transactions, correctly handle parameters:

1. If the message looks like a valid "create external", treat it as a contract creation.
2. If it looks like a valid EVM invocation, decode it as such.
3. Otherwise, ABI-encode the parameters to make them look like a "handle_filecoin_method" call. This
    will help chain explorers recognize these messages.

Part of #11355
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant