Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

FM-407: Insert 0x00..00 for system account #409

Merged
merged 3 commits into from
Nov 9, 2023
Merged

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Nov 8, 2023

Fixes consensus-shipyard/ipc#166

Inserts into the Init actor an association from the "null" f410 address 0x00..00 to the SYSTEM_ACTOR_ID. This way calls that do not set a from will continue to work with implicit semantics FWIW.

@raulk
Copy link
Contributor

raulk commented Nov 8, 2023

That's interesting. Doing this wouldn't work in Lotus, since I think we validate that the sender of an Eth message is nothing else but an Eth Account (EOA). It might just work here. You don't foresee any complications down the line in merging these types? You mentioned something about doing this to disable gas accounting. We do want gas accounting to run though, since I think some apps may call eth_estimateGas with the null sender.

@aakoshh
Copy link
Contributor Author

aakoshh commented Nov 8, 2023

I'm not sure but intuitively the masked ID address of the system actor also didn't come with an ethereum account actor attached to it, and it worked in calls. As far as I can see as long as the address can be turned into an ID it should be equivalent.

The implicit execution still counts the gas and respects the gas limit, it just doesn't check that the sender has enough balance to cover it up front, so it's one less thing to worry about with calls. If you send a call in your own name, then it's an explicit execution and you need to have the funds, however currently there is no miner penalty for including transactions with 0 gas price, so that's another way to circumvent this. But tools often estimate the price and set it, which sometimes means a lot of stars have to align.

@aakoshh
Copy link
Contributor Author

aakoshh commented Nov 8, 2023

It looks like you are right about there being some validation, the tests failed with:

2023-11-08T01:23:36.415823Z  INFO ethers: example transfer height=1088 tx_hash=0xb338e8cd3676f2fa6e49b3315199cf6baee9fc1599df03adc69752ede5e7b939
Error: failed to call eth_call: (code: 1, message: pre-validation failed: Send not from valid sender
, data: Some(String("")))


define_singleton!(SYSTEM { id: 0, code_id: 1 });

lazy_static! {
/// The Ethereum null-address 0x00..00 can also be used to identify the system actor.
Copy link
Contributor

Choose a reason for hiding this comment

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

@raulk, is this also the case in lotus?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on this I think they just create Ethereum account actors with these addresses, they aren't the same as the system address. It is only me who thought since we have this notion of sending these calls as the system actor, and that's actually what Lotus does too, then why not make it official?

@@ -190,7 +190,7 @@ where
msg.gas_limit = fvm_shared::BLOCK_GAS_LIMIT;
}

if msg.from == SYSTEM_ACTOR_ADDR {
if is_system_addr(&msg.from) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a dumb question, but what happens if a user sends an external transaction with the zero address in the from? I guess that it will have no signature and it will be rejected, right? I am just wondering if we are opening the door here to users being able to send free transactions that are executed implicitly by just setting from = 0x00..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, they would have to sign these transactions as well, which is impossible. It's the same as them setting an FVM transaction from to the f0 address of the system actor.

@aakoshh aakoshh merged commit 0169123 into main Nov 9, 2023
5 checks passed
@aakoshh aakoshh deleted the fm-407-default-eth-sender branch November 9, 2023 14:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create EthAccount at 0x00..00 + fix default sender address
3 participants