filecoin-project / lotus

Reference implementation of the Filecoin protocol, written in Go
https://lotus.filecoin.io/
Other
2.84k stars 1.26k forks source link

Transaction Hash Mismatch when deploying contract using Open Zeppelin Upgrades #9839

Closed scotthconner closed 1 year ago

scotthconner commented 1 year ago

Edited by @jennijuju

To mitigate this issue, we will expand the existing scope of the sqlite DB that is introduced for events indexing to cover the etherum tx id <> filecoin message CID mapping. We should introduce a configurable variable for node operators to choose if they wanna store this mapping or not, it should be disabled by default.

For the mapping, when we get an ethereum transaction, we reformat it into a "filecoin" message and get a filecoin CID. We need to keep that tx id <> Filecoin CID mapping.


I am trying to deploy an OpenZeppelin ERC1155 contract to FEVM wallaby using the UUPSUpgradeable proxy pattern, and Hardhat upgrades plugin. The code looks as follows:

const deployment = await upgrades.deployProxy(contract, preparedArguments);
        await deployment.deployed();`

I've had to override the provider to manually set the gas prices to prevent a legacy transaction error:

if (chainId.toString() === '31415') {
      // Wrap the provider so we can override fee data.
      const provider = new ethers.providers.FallbackProvider([ethers.provider], 1);
      provider.getFeeData = async () => FEE_DATA;
      // override the signer, connected to the provider with hardcoded fee data
      owner = new ethers.Wallet(process.env.MY_PRIVATE_KEY, provider);
    }

However, upon deployment I get an error around mismatch expected transaction hash. The hash that comes back is different than the one expected. I'm not certain if this is a post-deployment issue, and the contract is fine, or if this prevented the deployment altogether. It's worth noting that I am using a private key from ethereum, and not certain if that would cause a hash difference. In either case, this is currently blocking FEVM users from using deployable contracts like they would on ethereum, as far as I can tell,

$ npx hardhat deploy --network wallaby --contract KeyVault

==== GENIE, DEPLOY! ====

{
  "contract": "KeyVault",
  "force": false,
  "upgrade": false
}

=== SIGNER INFO ===

 Signer Network Chain ID: 31415
 Signer Wallet Address: 0x4E5d95F1D3d1b1FB4a169554A6bff1fD164ACa2c
 Signer Balance: 4998.9

=== CONTRACT INFO ===

 Input alias: KeyVault
 Factory Signer Chain ID: 31415
 Factory Signer Address:0x4E5d95F1D3d1b1FB4a169554A6bff1fD164ACa2c

=== NETWORK CONDITIONS ===

 Gas Price: 0.000200278

=== Contract Dependencies ===

Determined Dependencies:
[]

=== Deploying ... ===

Preparing dependency arguments...
Local Code Hash: 0x20b0e0d5ebf32eeee43f6d3bfda7015fee5aa7834abd54d91e2acf352f734987
Calling upgrades.deployProxy with #initialize([])
oooops!: Error: Transaction hash mismatch from Provider.sendTransaction. (expectedHash="0xf8888767ae4f58c6c6650b48ad28f7aa6b7390e13c033d541c8949cfaee2cd1d", returnedHash="0xb5df9c57a34a2807171473190d212f4f7db04da110d64d3534f6dca04d782fc2", code=UNKNOWN_ERROR, version=providers/5.7.0)
An unexpected error occurred:

**Error: Transaction hash mismatch from Provider.sendTransaction.** **(expectedHash="0xf8888767ae4f58c6c6650b48ad28f7aa6b7390e13c033d541c8949cfaee2cd1d", returnedHash="0xb5df9c57a34a2807171473190d212f4f7db04da110d64d3534f6dca04d782fc2",** code=UNKNOWN_ERROR, version=providers/5.7.0)
    at Logger.makeError (/Users/scotthconner/projects/smartrust/node_modules/@ethersproject/logger/src.ts/index.ts:269:28)
    at Logger.throwError (/Users/scotthconner/projects/smartrust/node_modules/@ethersproject/logger/src.ts/index.ts:281:20)
    at EthersProviderWrapper.BaseProvider._wrapTransaction (/Users/scotthconner/projects/smartrust/node_modules/@ethersproject/providers/src.ts/base-provider.ts:1522:20)
    at EthersProviderWrapper.<anonymous> (/Users/scotthconner/projects/smartrust/node_modules/@ethersproject/providers/src.ts/base-provider.ts:1569:25)
    at step (/Users/scotthconner/projects/smartrust/node_modules/@ethersproject/providers/lib/base-provider.js:48:23)
    at Object.next (/Users/scotthconner/projects/smartrust/node_modules/@ethersproject/providers/lib/base-provider.js:29:53)
    at fulfilled (/Users/scotthconner/projects/smartrust/node_modules/@ethersproject/providers/lib/base-provider.js:20:58)
    at processTicksAndRejections (node:internal/process/task_queues:95:5) {
  reason: 'Transaction hash mismatch from Provider.sendTransaction.',
  code: 'UNKNOWN_ERROR',
  expectedHash: '0xf8888767ae4f58c6c6650b48ad28f7aa6b7390e13c033d541c8949cfaee2cd1d',
  returnedHash: '0xb5df9c57a34a2807171473190d212f4f7db04da110d64d3534f6dca04d782fc2',
  transaction: {
    type: 2,
    chainId: 31415,
    nonce: 11,
    maxPriorityFeePerGas: BigNumber { value: "10000000000" },
    maxFeePerGas: BigNumber { value: "10000000000" },
    gasPrice: null,
    gasLimit: BigNumber { value: "181941066" },
    to: null,
    value: BigNumber { value: "0" },
    data: '0x60a0604052306080523480156200001557600080fd5b506200002062000026565b620000e8565b600054610100900460ff1615620000935760405162461bcd60e51b815260206004820152602760248201527f496e697469616c697a61626c653a20636f6e747261637420697320696e697469604482015266616c697a696e6760c81b606482015260840160405180910390fd5b60005460ff9081161015620000e6576000805460ff191660ff9081179091556040519081527f7f26b83ff96e1f2b6a682f133852f6798a09c465da95921460cefb38474024989060200160405180910390a15b565b6080516130f36200012060003960008181610869015281816108ee01528181610bdd01528181610c620152610d4c01526130f36000f3fe6080604052600436106101695760003560e01c80634e1273f4116100cb5780638f17ae321161007f578063e985e9c511610059578063e985e9c5146103fd578063f242432a14610446578063f5298aca1461046657600080fd5b80638f17ae321461038257806392707869146103b0578063a22cb465146103dd57600080fd5b806352d1902d116100b057806352d1902d14610338578063731133e91461034d5780638129fc1c1461036d57600080fd5b80634e1273f4146103055780634f1ef2861461032557600080fd5b80632e9d4d851161012257806334e80c341161010757806334e80c34146102985780633659cfe6146102c55780633faf3d6b146102e557600080fd5b80632e9d4d85146102565780632eb2c2d61461027857600080fd5b80630e89341c116101535780630e89341c146101d157806313fb817e146101fe5780631eba59aa1461023657600080fd5b8062fdd58e1461016e57806301ffc9a7146101a1575b600080fd5b34801561017a57600080fd5b5061018e6101893660046127b5565b610486565b6040519081526020015b60405180910390f35b3480156101ad57600080fd5b506101c16101bc3660046127f5565b610534565b6040519015158152602001610198565b3480156101dd57600080fd5b506101f16101ec366004612812565b6105cf565b604051610198919061287b565b34801561020a57600080fd5b5060fc5461021e906001600160a01b031681565b6040516001600160a01b039091168152602001610198565b34801561024257600080fd5b5061018e61025136600461289e565b610663565b34801561026257600080fd5b506102766102713660046128da565b610724565b005b34801561028457600080fd5b50610276610293366004612a41565b610799565b3480156102a457600080fd5b506102b86102b33660046128da565b61083b565b6040516101989190612b26565b3480156102d157600080fd5b506102766102e03660046128da565b61085f565b3480156102f157600080fd5b50610276610300366004612b39565b6109da565b34801561031157600080fd5b506102b8610320366004612b6c565b610a95565b610276610333366004612c37565b610bd3565b34801561034457600080fd5b5061018e610d3f565b34801561035957600080fd5b50610276610368366004612c7b565b610e05565b34801561037957600080fd5b50610276610eb7565b34801561038e57600080fd5b5061018e61039d366004612812565b6101006020526000908152604090205481565b3480156103bc57600080fd5b506103d06103cb366004612812565b610ff5565b6040516101989190612d0f565b3480156103e957600080fd5b506102766103f8366004612d5c565b61100f565b34801561040957600080fd5b506101c1610418366004612d8f565b6001600160a01b03918216600090815260666020908152604080832093909416825291909152205460ff1690565b34801561045257600080fd5b50610276610461366004612db9565b61101a565b34801561047257600080fd5b50610276610481366004612b39565b6110b5565b60006001600160a01b0383166105095760405162461bcd60e51b815260206004820152602a60248201527f455243313135353a2061646472657373207a65726f206973206e6f742061207660448201527f616c6964206f776e65720000000000000000000000000000000000000000000060648201526084015b60405180910390fd5b5060008181526065602090815260408083206001600160a01b03861684529091529020545b92915050565b60006001600160e01b031982167fd9b67a2600000000000000000000000000000000000000000000000000000000148061059757506001600160e01b031982167f0e89341c00000000000000000000000000000000000000000000000000000000145b8061052e57507f01ffc9a7000000000000000000000000000000000000000000000000000000006001600160e01b031983161461052e565b6060606780546105de90612e1e565b80601f016020809104026020016040519081016040528092919081815260200182805461060a90612e1e565b80156106575780601f1061062c57610100808354040283529160200191610657565b820191906000526020600020905b81548152906001019060200180831161063a57829003601f168201915b50505050509050919050565b6000816106f5576040517efdd58e0000000000000000000000000000000000000000000000000000000081526001600160a01b038516600482015260248101849052309062fdd58e90604401602060405180830381865afa1580156106cc573d6000803e3d6000fd5b505050506040513d601f19601f820116820180604052508101906106f09190612e58565b61071a565b6001600160a01b038416600090815260fd602090815260408083208684529091529020545b90505b9392505050565b60fb546001600160a01b0316331461076a5760405162461bcd60e51b81526020600482015260096024820152682727aa2fa7aba722a960b91b6044820152606401610500565b60fc805473ffffffffffffffffffffffffffffffffffffffff19166001600160a01b0392909216919091179055565b6001600160a01b0385163314806107b557506107b58533610418565b6108275760405162461bcd60e51b815260206004820152602f60248201527f455243313135353a2063616c6c6572206973206e6f7420746f6b656e206f776e60448201527f6572206e6f7220617070726f76656400000000000000000000000000000000006064820152608401610500565b6108348585858585611134565b5050505050565b6001600160a01b038116600090815260fe6020526040902060609061052e906113b8565b6001600160a01b037f00000000000000000000000000000000000000000000000000000000000000001630036108ec5760405162461bcd60e51b815260206004820152602c60248201527f46756e6374696f6e206d7573742062652063616c6c6564207468726f7567682060448201526b19195b1959d85d1958d85b1b60a21b6064820152608401610500565b7f00000000000000000000000000000000000000000000000000000000000000006001600160a01b03166109477f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc546001600160a01b031690565b6001600160a01b0316146109b25760405162461bcd60e51b815260206004820152602c60248201527f46756e6374696f6e206d7573742062652063616c6c6564207468726f7567682060448201526b6163746976652070726f787960a01b6064820152608401610500565b6109bb816113c5565b604080516000808252602082019092526109d79183919061140b565b50565b60fc546001600160a01b03163314610a245760405162461bcd60e51b815260206004820152600d60248201526c09c9ea8be989e8696a69a92a89609b1b6044820152606401610500565b6001600160a01b038316600081815260fd602090815260408083208684528252918290208490558151338152908101929092528101839052606081018290527fdd4ed73814d490cf0c19636b7b143543516fb68cc7d4fff60b44805e18913f449060800160405180910390a1505050565b60608151835114610b0e5760405162461bcd60e51b815260206004820152602960248201527f455243313135353a206163636f756e747320616e6420696473206c656e67746860448201527f206d69736d6174636800000000000000000000000000000000000000000000006064820152608401610500565b6000835167ffffffffffffffff811115610b2a57610b2a6128f5565b604051908082528060200260200182016040528015610b53578160200160208202803683370190505b50905060005b8451811015610bcb57610b9e858281518110610b7757610b77612e71565b6020026020010151858381518110610b9157610b91612e71565b6020026020010151610486565b828281518110610bb057610bb0612e71565b6020908102919091010152610bc481612e9d565b9050610b59565b509392505050565b6001600160a01b037f0000000000000000000000000000000000000000000000000000000000000000163003610c605760405162461bcd60e51b815260206004820152602c60248201527f46756e6374696f6e206d7573742062652063616c6c6564207468726f7567682060448201526b19195b1959d85d1958d85b1b60a21b6064820152608401610500565b7f00000000000000000000000000000000000000000000000000000000000000006001600160a01b0316610cbb7f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc546001600160a01b031690565b6001600160a01b031614610d265760405162461bcd60e51b815260206004820152602c60248201527f46756e6374696f6e206d7573742062652063616c6c6564207468726f7567682060448201526b6163746976652070726f787960a01b6064820152608401610500565b610d2f826113c5565b610d3b8282600161140b565b5050565b6000306001600160a01b037f00000000000000000000000000000000000000000000000000000000000000001614610ddf5760405162461bcd60e51b815260206004820152603860248201527f555550535570677261646561626c653a206d757374206e6f742062652063616c60448201527f6c6564207468726f7567682064656c656761746563616c6c00000000000000006064820152608401610500565b507f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc5b90565b60fc546001600160a01b03163314610e4f5760405162461bcd60e51b815260206004820152600d60248201526c09c9ea8be989e8696a69a92a89609b1b6044820152606401610500565b6000848152610100602052604081208054859290610e6e908490612eb6565b9250508190555061083485858585858080601f0160208091040260200160405190810160405280939291908181526020018383808284376000920191909152506115ab92505050565b600054610100900460ff1615808015610ed75750600054600160ff909116105b80610ef15750303b158015610ef1575060005460ff166001145b610f635760405162461bcd60e51b815260206004820152602e60248201527f496e697469616c697a61626c653a20636f6e747261637420697320616c72656160448201527f647920696e697469616c697a65640000000000000000000000000000000000006064820152608401610500565b6000805460ff191660011790558015610f86576000805461ff0019166101001790555b60fb805473ffffffffffffffffffffffffffffffffffffffff191633179055610fad6116ec565b80156109d7576000805461ff0019169055604051600181527f7f26b83ff96e1f2b6a682f133852f6798a09c465da95921460cefb38474024989060200160405180910390a150565b600081815260ff6020526040902060609061052e906113b8565b610d3b33838361176b565b6001600160a01b03851633148061103657506110368533610418565b6110a85760405162461bcd60e51b815260206004820152602f60248201527f455243313135353a2063616c6c6572206973206e6f7420746f6b656e206f776e60448201527f6572206e6f7220617070726f76656400000000000000000000000000000000006064820152608401610500565b610834858585858561185f565b60fc546001600160a01b031633146110ff5760405162461bcd60e51b815260206004820152600d60248201526c09c9ea8be989e8696a69a92a89609b1b6044820152606401610500565b600082815261010060205260408120805483929061111e908490612ec9565b9091555061112f9050838383611a1c565b505050565b81518351146111ab5760405162461bcd60e51b815260206004820152602860248201527f455243313135353a2069647320616e6420616d6f756e7473206c656e6774682060448201527f6d69736d617463680000000000000000000000000000000000000000000000006064820152608401610500565b6001600160a01b03841661120f5760405162461bcd60e51b815260206004820152602560248201527f455243313135353a207472616e7366657220746f20746865207a65726f206164604482015264647265737360d81b6064820152608401610500565b3361121e818787878787611be3565b60005b845181101561134a57600085828151811061123e5761123e612e71565b60200260200101519050600085838151811061125c5761125c612e71565b60209081029190910181'... 15640 more characters,
    accessList: [],
    hash: '0xf8888767ae4f58c6c6650b48ad28f7aa6b7390e13c033d541c8949cfaee2cd1d',
    v: 0,
    r: '0x163db65be4aed27f72485245b36be1e4f5c1e1e520fb20ef256d90f61c29b334',
    s: '0x6b255de6b9e9c32857a62e3e59514934639fb52abe6d523fcaf46acfbb4121da',
    from: '0x4E5d95F1D3d1b1FB4a169554A6bff1fD164ACa2c',
    confirmations: 0
  },
  transactionHash: '0xf8888767ae4f58c6c6650b48ad28f7aa6b7390e13c033d541c8949cfaee2cd1d'
Stebalien commented 1 year ago

I've had to override the provider to manually set the gas prices to prevent a legacy transaction error:

Is hardhat trying to use legacy transactions? Or is it falling back on them because we're missing something?

However, upon deployment I get an error around mismatch expected transaction hash.

I believe the issue is that we use the FVM message CID as the transaction hash to avoid having to keep a separate index. Some tools use the hash returned by the client, but apparently hardhat does not...

Unfortunately, there's no great solution here. We might just need to bite the bullet and keep an index, but that discussion will have to involve the lotus team.

scotthconner commented 1 year ago

It does look like the message was broadcast, just that ethers barfed because the transaction hash differed. Based on the block explorer, I cannot see if the deployment was successful, or not:

https://explorer.glif.io/tx/0xb5df9c57a34a2807171473190d212f4f7db04da110d64d3534f6dca04d782fc2/?network=wallabynet

scotthconner commented 1 year ago

It was trying to use a legacy format, but I've since fixed that issue by forcing the gas parameters. There was a note here: https://github.com/filecoin-project/FEVM-Hardhat-Kit/blob/main/deploy/00_deploy.js#L79

That lead me to fix that issue. The contract looks like it deploys, but it returns a different hash than what ethers is expecting. You can see in the link above that the EVM transaction hash is correct: "0xb5df9c57a34a2807171473190d212f4f7db04da110d64d3534f6dca04d782fc2". This matches what is in the block explorer: https://explorer.glif.io/tx/0xb5df9c57a34a2807171473190d212f4f7db04da110d64d3534f6dca04d782fc2/?network=wallabynet

However, the expected hash on my client is: "0xf8888767ae4f58c6c6650b48ad28f7aa6b7390e13c033d541c8949cfaee2cd1d"

So my intuition is that, somewhere in the FEVM, they are doing something different that changes the hash. Maybe it is using the CID, or the "t-address" of t410fjzozl4ot2gy7wsqwsvkknp7r7ulevsrmdvea6pq to hash instead of the EVM address 0x4E5d95F1D3d1b1FB4a169554A6bff1fD164ACa2c. On the client side, the "t-address" was not visible, as the wallet was derived directly from the private key.

I'm wondering if the "from" address is using t410fiz... on the node, and 0x4E5.... on the client, thus the mismatch.

In either case, the hash is coming back different and preventing my deployment script from finishing (for instance, its not even clear to me if the proxy was properly initialized).

scotthconner commented 1 year ago

Before dragging in Lotus and signing up for an index, is there any help you can provide me in helping determine why the hash is different? If it is indeed due to CID, then we can work through that. It may just require some extension of ethers.js, but I can't be sure.

Stebalien commented 1 year ago

Basically, when we get an ethereum transaction, we have to reformat it into a "filecoin" message. Then we get a filecoin CID. We then return the filecoin message CID's digest as the "tx hash".

See https://github.com/filecoin-project/lotus/blob/f681a350fcc9c39a3022a6708fa5da777dd83efa/node/impl/full/eth.go#L516

Stebalien commented 1 year ago

Ideally, eithers would use the txn hash lotus returns and just emit a warning if it doesn't match.

scotthconner commented 1 year ago

Thanks, I can see where the CID is basically used to produce the new filecoin hash.

Problem is, out of the box, ethers doesn't use the lotus hash, and right now its not entirely possible to simply ignore the mismatch. ethers throws an exception, and the script can't get the deployed contract's address to do anything with. Because of this, I'm not certain that the proxy deployment ever truly finishes.

This behavior is likely to break any ethereum client-side implementations that do not anticipate a new type of transaction hash logic outside of the standard EVM. I'm not certain how to use existing ethereum toolchains to develop and deploy upgradeable contracts. Not being able to use ethers for FVM client-side development is going to limit developer portability from the Ethereum ecosystem.

Are upgradeable contracts in scope for the first release? Are they scoped into a subsequent release?

scotthconner commented 1 year ago

Do we know why we return the hash of the CID for FEVM, versus just the Ethereum transaction hash?

Stebalien commented 1 year ago

Problem is, out of the box, ethers doesn't use the lotus hash, and right now its not entirely possible to simply ignore the mismatch. ethers throws an exception, and the script can't get the deployed contract's address to do anything with. Because of this, I'm not certain that the proxy deployment ever truly finishes.

Most tools will use the hash as returned by EthSendRawTransaction.

Do we know why we return the hash of the CID for FEVM, versus just the Ethereum transaction hash?

If we just hashed the transaction, we'd have no way to look it up without an index.

Stebalien commented 1 year ago

Or, at least, remix and metamask appear to work.

scotthconner commented 1 year ago

I'm not sure I understand why there would need to be an index. The block explorer seems capable of displaying both the CID, and the ethereum transaction hash. How does glif get the ethereum transaction hash?

I'm also unsure why we have to return the CID hashed - what "breaks" if we return what ethereum would have in that scenario?

scotthconner commented 1 year ago

In looking at this line here: https://github.com/filecoin-project/lotus/blob/f681a350fcc9c39a3022a6708fa5da777dd83efa/node/impl/full/eth.go#L516

That this line was touched 26 days ago trivially: https://github.com/filecoin-project/lotus/pull/9631

But the real introduction was in response to FIP-422.

I think I'm grokking why the transaction is returning a hash of a CID (a CID being something standard EVM-based tools know nothing about) based on the implications in your comment here.

Interface-wise, on the toolchain side the caller should be able to anticipate the resulting transaction hash.

I'm going to keep digging to ensure I'm not missing something in my transaction signature or doing something weird.

scotthconner commented 1 year ago

After hacking in ethers.js for a while, I've had to:

As far as I can tell, FEVM returns its own transaction hash schema and is generally incompatible with the way ether.js does some sendTransactions, because it produces the hash on the client side and expects the node to return the same (as a security measure, I would imagine).

snissn commented 1 year ago

having the hash match sounds really crucial - it's a common eth coding paradigm to make a tx then poll for it to be confirmed

Is this how to go from a fil cid to an eth tx hash? https://github.com/filecoin-project/lotus/blob/f681a350fcc9c39a3022a6708fa5da777dd83efa/api/eth_types.go#L415

it makes a lot of sense to me to have the eth RPC return the eth tx hash

scotthconner commented 1 year ago

@snissn that method is actually used to return an "eth-like" transaction hash. You can see where it is used here: https://github.com/filecoin-project/lotus/blob/f681a350fcc9c39a3022a6708fa5da777dd83efa/node/impl/full/eth.go#L516

However, it looks like a keccak256 hash of the resulting CID. While the hash "looks" like an ethereum hash, it is not the ethereum hash that uses the same hashing algorithm to produce the transaction hash from the payload. The transaction hash generated by that method using the Filecoin CID, which, by its nature, will always result in a different transaction hash. More importantly, existing ethereum client-side toolchains (like ethers), won't be able to derive the CID to anticipate the hash without non-trivial toolchain development specific for FEVM. If we want robust compatibility between ecosystems, we should produce a transaction hash the same way the standard EVM chains do.

snissn commented 1 year ago

the tx hash on ethereum fortunately is simple it's just the keccak 256 hash of the raw tx Here's a simple gist that submits a tx to wallaby. https://gist.github.com/snissn/b0418759bea03d1ceca8aba7410213c2 It prints out the signedTx, and if you put that larger string into https://emn178.github.io/online-tools/keccak_256.html (delete 0x and select hex) you can recreate the expected hash.

The gist code ends up throwing an exception with reason: 'Transaction hash mismatch from Provider.sendTransaction.', the expected return from the wallaby rpc is the keccak 256 hash of the signedTx. because the rpc endoint returns the hash of the cid (not the hash of the raw tx)

I think if it doesn't break everything, and my assumption about the data / code is correct having EthSendRawTransaction return the hash(rawTx) (rawTx the input of the fn you linked to) may/should give us the result we need. however i don't know what depdendences exist on that endpoint

scotthconner commented 1 year ago

Fully aligned @snissn. Let's see what @Stebalien thinks, as I think he has visibility into use cases that might make those more difficult - but we'll need to consider impacts and trade-offs.

ychiaoli18 commented 1 year ago

I'm also unsure why we have to return the CID hashed - what "breaks" if we return what ethereum would have in that scenario?

Filecoin's Eth-style JSON-RPC relies on Cid to retrieve the objects such as blocks or txs, and it simply wraps a hash into a Cid or unwraps a Cid to a hash. If we want to generate the hash in an Ethereum-compatible way now, the endpoint would not be able to get the correct Cid and will not be able to retrieve the objects from the blockstore (see here).

I agree that it's important to generate the hash in an Ethereum-compatible way, and we may have to maintain an index. @arajasek do you think this is the best way to move forward?

raulk commented 1 year ago

We do return an Eth looking transaction hash. This is the hex encoding of the rightmost 32 bytes of the multihash hash within the message CID. Lotus computes the message CID off the message that has been pushed to the mempool, which is the Filecoin delegated-signature message in this case (not of the original Eth transaction bytes!). This uses neither the preimage expected by Eth clients, nor the hash function (keccak256 vs blake2b). This is where the mismatch lies.

Tools and libraries like ethers.js probably calculate the hash of the transaction to avoid placing trust at the endpoint. If they blindly used the hash returned by eth_sendRawTransaction, a malicious endpoint could spoof the hash and trick the client into tracking a different transaction, with all kinds of deleterious application-dependent downstream effects.

Unfortunately, I can't think of a reasonable way of solving this problem without an index.

The other solution I could come up with highly depends on the final shape of account abstraction. Particularly, whether we embed or not the original tx bytes in the delegated-signature message. If we did, we could add the original Eth tx to the blockstore with a keccak256 multihash, and have the delegated-signature message link to the Eth tx block by CID, instead of inlining it. However, that would solve one problem (returning the right tx hash), but lookups would still be unsolved unless we store some other metadata, i.e. how do we go from an Eth tx hash to the Filecoin message, for tracking purposes?

Alternatively we could advocate for having all incompatible Eth libraries and tools adapted so we could plug in the message hash calculation logic (and we'd have to produce plugins for each to integrate our logic). But this sounds frankly unrealistic.

scotthconner commented 1 year ago

Thanks @raulk for the insight. I figured the mapping of the eth hash to the Filecoin blockstore was likely the reason - and the current hashing scheme allows for a two way street from hash to CID. The clients, as you've also stated, likely pre-compute the hash to ensure that RPC endpoints don't have to be trusted. This is a good design.

I agree that it looks like some mapping, or index, might needed to solve it, unless Filecoin can somehow find a way to have both types of hashes, although I'd imagine this would make things much more complex within the node implementations themselves, if feasible at all.

I also consider the need for every toolchain in the Ethereum development environment to adapt to the way Filecoin does hashing to be unrealistic. It also is likely a game of catch-up, every time a new tool is built for Ethereum its likely not to support FEVM in this way, and it becomes a game of whack-a-mole.

Considering the purpose of the FEVM is to capture interest of Ethereum Solidity developers and provide a familiar experience, allowing them to use Ethers.js, Web3.js, and other clients to interact with the blockchain are a huge win for developer on-boarding.

I'll let @arajasek chime in as well. Thanks for explaining some of the details I only partially understood.

jennijuju commented 1 year ago

I am moving this issue to lotus as this touches client more than ref-fvm. We will create an new issue if account abstraction impl update is needed. Whoever is creating the FRC for the eth_rpc client support should capture this, so that other implementations(venus and forest) can also mitigate this.

raulk commented 1 year ago

@jennijuju hm, please discuss issue management with @maciejwitowski. We track all FVM items in ref-fvm even if they solely impact the client.

jennijuju commented 1 year ago

@raulk we have identified this as a gap and have started to move client issues to its right home. With this, two teams should also talk to each other more often on over all nv18 (which include not only ref-fvm but also client- lotus) progress, which was missing earlier.

scotthconner commented 1 year ago

@jennijuju what are the implications of not having/storing this index by default? Does this mean that solidity/EVM developers would need to either run, or find, a node that supports proper ethereum transaction hashes? If a developer deploys a contract to a node that doesn't have this index, do they still get a proper eth transaction back? If so, can they not poll against the receipt on the same node? What about block explorers?