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

[env] dev to use anvil instead of geth for evm chains #1544

Merged
merged 26 commits into from
Feb 14, 2025
Merged

Conversation

agryaznov
Copy link
Contributor

@agryaznov agryaznov commented Feb 11, 2025

Closes #1543
Closes #1548

@agryaznov agryaznov marked this pull request as ready for review February 11, 2025 19:31
@agryaznov agryaznov requested a review from dvc94ch February 11, 2025 19:31
@dvc94ch
Copy link
Collaborator

dvc94ch commented Feb 11, 2025

Thx!

@agryaznov
Copy link
Contributor Author

Note this also would require some logic change reg. admin account funding, as connector's faucet woudn't work with anvil

2025-02-11T20:08:25.964414Z ERROR rosetta_server::ws::reconnect: rpc request 'eth_sendTransaction' failed: Call(ErrorObject { code: ServerError(-32003), message: "Insufficient funds for gas * price + value", data: None })
Call(
    ErrorObject {
        code: ServerError(
            -32003,
        ),
        message: "Insufficient funds for gas * price + value",
        data: None,
    },
)

@agryaznov agryaznov marked this pull request as draft February 11, 2025 20:17
@dvc94ch
Copy link
Collaborator

dvc94ch commented Feb 12, 2025

So this needs some more work:

  1. chain connector should be able to identify when dealing with anvil using the admin_nodeInfo rpc
  2. chain connector should use anvil_setBalance in the faucet

@dvc94ch
Copy link
Collaborator

dvc94ch commented Feb 12, 2025

never mind, the whole connector codebase is one giant hack. one more hack won't hurt...

@dvc94ch dvc94ch marked this pull request as ready for review February 12, 2025 11:32
@agryaznov
Copy link
Contributor Author

So this needs some more work:

1. chain connector should be able to identify when dealing with anvil using the `admin_nodeInfo` rpc

2. chain connector should use `anvil_setBalance` in the faucet

I think this approach is better than a dirty hack proposed here , with a minor changes:

  1. let's do it in chain connector by passing blockchain: "anvil" to params
  2. Then for anvil we pass private key of default dev account here

- '--verbosity=4'
- 'anvil -b 2 --steps-tracing --base-fee 0'
environment:
ANVIL_IP_ADDR: '0.0.0.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

which port does it export the rpc to host machine for querying?

Copy link
Collaborator

Choose a reason for hiding this comment

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

none. but you can query it with docker compose run tc-cli debug-transaction NETWORK_ID TX_HASH

@dvc94ch dvc94ch merged commit f52fff2 into development Feb 14, 2025
13 of 15 checks passed
@dvc94ch dvc94ch deleted the ag/anvil branch February 14, 2025 17:24
Copy link
Collaborator

@dvc94ch dvc94ch left a comment

Choose a reason for hiding this comment

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

@@ -17,7 +17,7 @@ interface IGmpReceiver {
* @param payload The message payload with no specified format
* @return 32 byte result which will be stored together with GMP message
*/
function onGmpReceived(bytes32 id, uint128 network, bytes32 source, bytes calldata payload)
function onGmpReceived(bytes32 id, uint128 network, bytes32 source, uint64 nonce, bytes calldata payload)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@agryaznov here's the change causing the issue

Comment on lines -39 to +55
function onGmpReceived(bytes32 id, uint128, bytes32, bytes calldata payload) external payable returns (bytes32) {
// For testing purpose
// we keep the original struct in payload so we dont depend on OnGmpReceived call since it doesnt provide everything.
(GmpMessage memory message) = abi.decode(payload, (GmpMessage));
message.data = payload;

function onGmpReceived(bytes32 id, uint128 srcNetwork, bytes32 src, uint64 nonce, bytes calldata payload) external payable returns (bytes32) {
// when estimating gas an insane amount of gas is provided
uint256 gasLimit = gasleft();
// this is the constant added to gasLimit
unchecked { console.log(300_000 - gasLimit); }
uint64 msgGasLimit;
unchecked { msgGasLimit = uint64(gasLimit + 579); }
GmpMessage memory message = GmpMessage({
source: src,
srcNetwork: uint16(srcNetwork),
dest: address(this),
destNetwork: NETWORK_ID,
gasLimit: msgGasLimit,
nonce: nonce,
data: payload
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

here's the reason for the change.

  1. nonce is determined by gateway
  2. this was worked around by guessing the nonce before submitting the message and then encoding the GmpMessage struct in the payload.

Comment on lines +701 to +720
async fn estimate_message_gas_limit(
&self,
contract: Address,
src_network: NetworkId,
src: Address,
payload: Vec<u8>,
) -> Result<u128> {
let call = sol::IGmpReceiver::onGmpReceivedCall {
id: [0; 32].into(),
network: src_network.into(),
source: src.into(),
nonce: 0,
payload: payload.into(),
};
let gas_limit = self
.wallet
.eth_send_call_estimate_gas(a_addr(contract).into(), call.abi_encode(), 0)
.await?;
Ok(gas_limit)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

we also guessed the gas_limit, which was hard coded to 300000 gas. this caused messages to be 5x overpriced

Comment on lines +722 to 742
async fn estimate_message_cost(
&self,
gateway: Address,
dest_network: NetworkId,
gas_limit: u128,
payload: Vec<u8>,
) -> Result<u128> {
let msg = sol::GmpMessage {
source: [0; 32].into(),
srcNetwork: 0,
dest: [0; 20].into(),
destNetwork: 0,
gasLimit: 0,
nonce: 0,
data: payload.into(),
};
let call = sol::Gateway::estimateMessageCostCall {
networkid: dest,
messageSize: U256::from(bytes.len()),
networkid: dest_network,
// abi_encoded_size returns the size without the 4 byte selector
messageSize: U256::from(msg.abi_encoded_size() + 4),
gasLimit: U256::from(gas_limit),
Copy link
Collaborator

Choose a reason for hiding this comment

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

this message used to be called internally in submit message, but we want to estimate the final cost before submitting the message

Comment on lines +417 to +423
let gas_limit = tc
.estimate_message_gas_limit(dest, dest_addr, src, src_addr, payload.clone())
.await?;
let gas_cost = tc.estimate_message_cost(src, dest, gas_limit, payload.clone()).await?;
let msg_id = tc
.send_message(src, src_addr, dest, dest_addr, gas_limit, gas_cost, payload.clone())
.await?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

see here for how its used

Comment on lines +361 to +386
Command::EstimateMessageGasLimit {
dest_network,
dest_addr,
src_network,
src_addr,
payload,
} => {
let src_addr = tc.parse_address(Some(src_network), &src_addr)?;
let dest_addr = tc.parse_address(Some(dest_network), &dest_addr)?;
let payload = hex::decode(payload)?;
let gas_limit = tc
.estimate_message_gas_limit(dest_network, dest_addr, src_network, src_addr, payload)
.await?;
tc.println(None, gas_limit.to_string()).await?;
},
Command::EstimateMessageGasCost {
src_network,
dest_network,
gas_limit,
payload,
} => {
let payload = hex::decode(payload)?;
let gas_cost =
tc.estimate_message_cost(src_network, dest_network, gas_limit, payload).await?;
tc.println(None, gas_cost.to_string()).await?;
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

and the new utilities we have in tc-cli

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants