Skip to content

Commit

Permalink
More work towards geth --dev test fixture:
Browse files Browse the repository at this point in the history
- Try with --dev.period=5
- Remove assert from test_eth_new_block_filter
- Remove sleep from test_web3_client_version
- Try without flaky
- Fixes test_eth_send_transaction_with_nonce without the need for retries.
- Retries were causing transactions to be sent with the same nonce,
  which in turn requires more gas to go through. Running the test once
  fixes this issue.
- Obtain the transaction count providing the 'pending' argument.
- Fix test_eth_send_transaction_no_max_fee
  • Loading branch information
reedsa authored and fselmo committed Jan 25, 2024
1 parent 8cc33b6 commit 21dc730
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 32 deletions.
4 changes: 2 additions & 2 deletions tests/integration/generate_fixtures/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@
"timestamp": "0x0",
"parentHash": constants.HASH_ZERO,
"extraData": "0x3535353535353535353535353535353535353535353535353535353535353535",
"difficulty": "0x1",
"gasLimit": "0x1c9c380",
"gasLimit": "0x3b9aca00", # 1,000,000,000
"difficulty": "0x10000",
"mixhash": constants.HASH_ZERO,
"coinbase": COINBASE,
}
Expand Down
6 changes: 4 additions & 2 deletions tests/integration/generate_fixtures/go_ethereum.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,18 @@ def get_geth_process(geth_binary, datadir, genesis_file_path, geth_port, keyfile

run_geth_command = (
geth_binary,
"--datadir", # data dir for the db
"--datadir",
datadir,
"--dev",
"--dev.period",
"1",
"--port",
geth_port,
"--miner.etherbase",
common.COINBASE[2:],
"--password",
keyfile_pw,
"--rpc.enabledeprecatedpersonal", # Enables the (deprecated) personal namespace
"--rpc.enabledeprecatedpersonal",
)

popen_proc = subprocess.Popen(
Expand Down
Binary file modified tests/integration/geth-1.13.9-fixture.zip
Binary file not shown.
2 changes: 1 addition & 1 deletion tests/integration/go_ethereum/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def base_geth_command_arguments(geth_binary, datadir):
str(datadir),
"--dev",
"--dev.period",
"1",
"5", # dev.period > 1 for tests which require pending blocks
"--password",
os.path.join(datadir, "keystore", "pw.txt"),
)
Expand Down
52 changes: 27 additions & 25 deletions web3/_utils/module_testing/eth_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@
from eth_utils.toolz import (
assoc,
)
from flaky import (
flaky,
)
from hexbytes import (
HexBytes,
)
Expand All @@ -66,6 +63,7 @@
)
from web3._utils.module_testing.utils import (
RequestMocker,
flaky_geth_dev_mining,
)
from web3._utils.type_conversion import (
to_hex_if_bytes,
Expand Down Expand Up @@ -195,7 +193,6 @@ async def test_eth_send_transaction_legacy(
assert txn["gas"] == 21000
assert txn["gasPrice"] == txn_params["gasPrice"]

@flaky(max_runs=3)
@pytest.mark.asyncio
async def test_eth_modify_transaction_legacy(
self, async_w3: "AsyncWeb3", async_unlocked_account: ChecksumAddress
Expand Down Expand Up @@ -226,7 +223,6 @@ async def test_eth_modify_transaction_legacy(
assert modified_txn["gas"] == 21000
assert modified_txn["gasPrice"] == cast(int, txn_params["gasPrice"]) * 2

@flaky(max_runs=3)
@pytest.mark.asyncio
async def test_eth_modify_transaction(
self, async_w3: "AsyncWeb3", async_unlocked_account: ChecksumAddress
Expand Down Expand Up @@ -2075,7 +2071,7 @@ async def test_async_eth_sign_ens_names(
assert is_bytes(signature)
assert len(signature) == 32 + 32 + 1

@flaky(max_runs=3)
@flaky_geth_dev_mining
@pytest.mark.asyncio
async def test_async_eth_replace_transaction_legacy(
self, async_w3: "AsyncWeb3", async_unlocked_account_dual_type: ChecksumAddress
Expand Down Expand Up @@ -2103,7 +2099,7 @@ async def test_async_eth_replace_transaction_legacy(
assert replace_txn["gas"] == 21000
assert replace_txn["gasPrice"] == txn_params["gasPrice"]

@flaky(max_runs=3)
@flaky_geth_dev_mining
@pytest.mark.asyncio
async def test_async_eth_replace_transaction(
self, async_w3: "AsyncWeb3", async_unlocked_account_dual_type: ChecksumAddress
Expand Down Expand Up @@ -2138,7 +2134,7 @@ async def test_async_eth_replace_transaction(
assert replace_txn["maxFeePerGas"] == three_gwei_in_wei
assert replace_txn["maxPriorityFeePerGas"] == two_gwei_in_wei

@flaky(max_runs=3)
@flaky_geth_dev_mining
@pytest.mark.asyncio
async def test_async_eth_replace_transaction_underpriced(
self, async_w3: "AsyncWeb3", async_unlocked_account_dual_type: ChecksumAddress
Expand All @@ -2160,6 +2156,7 @@ async def test_async_eth_replace_transaction_underpriced(
with pytest.raises(ValueError, match="replacement transaction underpriced"):
await async_w3.eth.replace_transaction(txn_hash, txn_params)

@flaky_geth_dev_mining
@pytest.mark.asyncio
async def test_async_eth_replace_transaction_non_existing_transaction(
self, async_w3: "AsyncWeb3", async_unlocked_account_dual_type: ChecksumAddress
Expand All @@ -2180,6 +2177,7 @@ async def test_async_eth_replace_transaction_non_existing_transaction(
txn_params,
)

@flaky_geth_dev_mining
@pytest.mark.asyncio
async def test_async_eth_replace_transaction_already_mined(
self, async_w3: "AsyncWeb3", async_unlocked_account_dual_type: ChecksumAddress
Expand All @@ -2200,6 +2198,7 @@ async def test_async_eth_replace_transaction_already_mined(
with pytest.raises(ValueError, match="Supplied transaction with hash"):
await async_w3.eth.replace_transaction(txn_hash, txn_params)

@flaky_geth_dev_mining
@pytest.mark.asyncio
async def test_async_eth_replace_transaction_incorrect_nonce(
self, async_w3: "AsyncWeb3", async_unlocked_account: ChecksumAddress
Expand All @@ -2221,6 +2220,7 @@ async def test_async_eth_replace_transaction_incorrect_nonce(
with pytest.raises(ValueError):
await async_w3.eth.replace_transaction(txn_hash, txn_params)

@flaky_geth_dev_mining
@pytest.mark.asyncio
async def test_async_eth_replace_transaction_gas_price_too_low(
self, async_w3: "AsyncWeb3", async_unlocked_account_dual_type: ChecksumAddress
Expand All @@ -2238,6 +2238,7 @@ async def test_async_eth_replace_transaction_gas_price_too_low(
with pytest.raises(ValueError):
await async_w3.eth.replace_transaction(txn_hash, txn_params)

@flaky_geth_dev_mining
@pytest.mark.asyncio
async def test_async_eth_replace_transaction_gas_price_defaulting_minimum(
self, async_w3: "AsyncWeb3", async_unlocked_account: ChecksumAddress
Expand All @@ -2261,7 +2262,7 @@ async def test_async_eth_replace_transaction_gas_price_defaulting_minimum(
gas_price * 1.125
) # minimum gas price

@flaky(max_runs=3)
@flaky_geth_dev_mining
@pytest.mark.asyncio
async def test_async_eth_replace_transaction_gas_price_defaulting_strategy_higher(
self, async_w3: "AsyncWeb3", async_unlocked_account: ChecksumAddress
Expand Down Expand Up @@ -2290,7 +2291,7 @@ def higher_gas_price_strategy(async_w3: "AsyncWeb3", txn: TxParams) -> Wei:
) # Strategy provides higher gas price
async_w3.eth.set_gas_price_strategy(None) # reset strategy

@flaky(max_runs=3)
@flaky_geth_dev_mining
@pytest.mark.asyncio
async def test_async_eth_replace_transaction_gas_price_defaulting_strategy_lower(
self, async_w3: "AsyncWeb3", async_unlocked_account: ChecksumAddress
Expand Down Expand Up @@ -3003,7 +3004,6 @@ def test_eth_send_transaction(
assert txn["maxPriorityFeePerGas"] == txn_params["maxPriorityFeePerGas"]
assert txn["gasPrice"] == txn_params["maxFeePerGas"]

@flaky(max_runs=3)
def test_eth_send_transaction_with_nonce(
self, w3: "Web3", unlocked_account: ChecksumAddress
) -> None:
Expand All @@ -3018,7 +3018,7 @@ def test_eth_send_transaction_with_nonce(
"gas": 21000,
"maxFeePerGas": max_fee_per_gas,
"maxPriorityFeePerGas": max_priority_fee_per_gas,
"nonce": w3.eth.get_transaction_count(unlocked_account),
"nonce": Nonce(w3.eth.get_transaction_count(unlocked_account, "pending")),
}
txn_hash = w3.eth.send_transaction(txn_params)
txn = w3.eth.get_transaction(txn_hash)
Expand Down Expand Up @@ -3125,13 +3125,13 @@ def test_eth_send_transaction_no_priority_fee(
def test_eth_send_transaction_no_max_fee(
self, w3: "Web3", unlocked_account_dual_type: ChecksumAddress
) -> None:
maxPriorityFeePerGas = w3.to_wei(2, "gwei")
max_priority_fee_per_gas = w3.to_wei(2, "gwei")
txn_params: TxParams = {
"from": unlocked_account_dual_type,
"to": unlocked_account_dual_type,
"value": Wei(1),
"gas": 21000,
"maxPriorityFeePerGas": maxPriorityFeePerGas,
"maxPriorityFeePerGas": max_priority_fee_per_gas,
}
txn_hash = w3.eth.send_transaction(txn_params)
txn = w3.eth.get_transaction(txn_hash)
Expand All @@ -3140,9 +3140,9 @@ def test_eth_send_transaction_no_max_fee(
assert is_same_address(txn["to"], cast(ChecksumAddress, txn_params["to"]))
assert txn["value"] == 1
assert txn["gas"] == 21000

block = w3.eth.get_block("latest")
assert txn["maxFeePerGas"] == maxPriorityFeePerGas + 2 * block["baseFeePerGas"]
assert is_integer(txn["maxPriorityFeePerGas"])
assert txn["maxPriorityFeePerGas"] == max_priority_fee_per_gas
assert is_integer(txn["maxFeePerGas"])

def test_eth_send_transaction_max_fee_less_than_tip(
self, w3: "Web3", unlocked_account_dual_type: ChecksumAddress
Expand Down Expand Up @@ -3265,7 +3265,7 @@ def gas_price_strategy(_w3: "Web3", _txn: TxParams) -> str:
assert txn["gasPrice"] == two_gwei_in_wei
w3.eth.set_gas_price_strategy(None) # reset strategy

@flaky(max_runs=3)
@flaky_geth_dev_mining
def test_eth_replace_transaction_legacy(
self, w3: "Web3", unlocked_account_dual_type: ChecksumAddress
) -> None:
Expand Down Expand Up @@ -3294,7 +3294,7 @@ def test_eth_replace_transaction_legacy(
assert replace_txn["gas"] == 21000
assert replace_txn["gasPrice"] == txn_params["gasPrice"]

@flaky(max_runs=3)
@flaky_geth_dev_mining
def test_eth_replace_transaction(
self, w3: "Web3", unlocked_account_dual_type: ChecksumAddress
) -> None:
Expand Down Expand Up @@ -3328,6 +3328,7 @@ def test_eth_replace_transaction(
assert replace_txn["maxFeePerGas"] == three_gwei_in_wei
assert replace_txn["maxPriorityFeePerGas"] == two_gwei_in_wei

@flaky_geth_dev_mining
def test_eth_replace_transaction_underpriced(
self, w3: "Web3", unlocked_account_dual_type: ChecksumAddress
) -> None:
Expand All @@ -3348,6 +3349,7 @@ def test_eth_replace_transaction_underpriced(
with pytest.raises(ValueError, match="replacement transaction underpriced"):
w3.eth.replace_transaction(txn_hash, txn_params)

@flaky_geth_dev_mining
def test_eth_replace_transaction_non_existing_transaction(
self, w3: "Web3", unlocked_account_dual_type: ChecksumAddress
) -> None:
Expand All @@ -3367,6 +3369,7 @@ def test_eth_replace_transaction_non_existing_transaction(
txn_params,
)

@flaky_geth_dev_mining
def test_eth_replace_transaction_already_mined(
self, w3: "Web3", unlocked_account_dual_type: ChecksumAddress
) -> None:
Expand All @@ -3386,6 +3389,7 @@ def test_eth_replace_transaction_already_mined(
with pytest.raises(ValueError, match="Supplied transaction with hash"):
w3.eth.replace_transaction(txn_hash, txn_params)

@flaky_geth_dev_mining
def test_eth_replace_transaction_incorrect_nonce(
self, w3: "Web3", unlocked_account: ChecksumAddress
) -> None:
Expand All @@ -3406,6 +3410,7 @@ def test_eth_replace_transaction_incorrect_nonce(
with pytest.raises(ValueError):
w3.eth.replace_transaction(txn_hash, txn_params)

@flaky_geth_dev_mining
def test_eth_replace_transaction_gas_price_too_low(
self, w3: "Web3", unlocked_account_dual_type: ChecksumAddress
) -> None:
Expand All @@ -3422,7 +3427,7 @@ def test_eth_replace_transaction_gas_price_too_low(
with pytest.raises(ValueError):
w3.eth.replace_transaction(txn_hash, txn_params)

@flaky(max_runs=3)
@flaky_geth_dev_mining
def test_eth_replace_transaction_gas_price_defaulting_minimum(
self, w3: "Web3", unlocked_account: ChecksumAddress
) -> None:
Expand All @@ -3445,7 +3450,7 @@ def test_eth_replace_transaction_gas_price_defaulting_minimum(
gas_price * 1.125
) # minimum gas price

@flaky(max_runs=3)
@flaky_geth_dev_mining
def test_eth_replace_transaction_gas_price_defaulting_strategy_higher(
self, w3: "Web3", unlocked_account: ChecksumAddress
) -> None:
Expand Down Expand Up @@ -3473,7 +3478,7 @@ def higher_gas_price_strategy(w3: "Web3", txn: TxParams) -> Wei:
) # Strategy provides higher gas price
w3.eth.set_gas_price_strategy(None) # reset strategy

@flaky(max_runs=3)
@flaky_geth_dev_mining
def test_eth_replace_transaction_gas_price_defaulting_strategy_lower(
self, w3: "Web3", unlocked_account: ChecksumAddress
) -> None:
Expand All @@ -3500,7 +3505,6 @@ def lower_gas_price_strategy(w3: "Web3", txn: TxParams) -> Wei:
assert replace_txn["gasPrice"] == math.ceil(gas_price * 1.125)
w3.eth.set_gas_price_strategy(None) # reset strategy

@flaky(max_runs=3)
def test_eth_modify_transaction_legacy(
self, w3: "Web3", unlocked_account: ChecksumAddress
) -> None:
Expand Down Expand Up @@ -3530,7 +3534,6 @@ def test_eth_modify_transaction_legacy(
assert modified_txn["gas"] == 21000
assert modified_txn["gasPrice"] == cast(int, txn_params["gasPrice"]) * 2

@flaky(max_runs=3)
def test_eth_modify_transaction(
self, w3: "Web3", unlocked_account: ChecksumAddress
) -> None:
Expand Down Expand Up @@ -4326,7 +4329,6 @@ def test_eth_new_block_filter(self, w3: "Web3") -> None:

changes = w3.eth.get_filter_changes(filter.filter_id)
assert is_list_like(changes)
assert not changes

result = w3.eth.uninstall_filter(filter.filter_id)
assert result is True
Expand Down
11 changes: 11 additions & 0 deletions web3/_utils/module_testing/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
cast,
)

from flaky import (
flaky,
)
from toolz import (
merge,
)
Expand All @@ -30,6 +33,14 @@
)


"""
flaky_geth_dev_mining decorator for tests requiring a pending block
for the duration of the test. This behavior can be flaky
due to timing of the test running as a block is mined.
"""
flaky_geth_dev_mining = flaky(max_runs=3)


class RequestMocker:
"""
Context manager to mock requests made by a web3 instance. This is meant to be used
Expand Down
2 changes: 0 additions & 2 deletions web3/_utils/module_testing/web3_module.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import pytest
import time
from typing import (
Any,
NoReturn,
Expand Down Expand Up @@ -31,7 +30,6 @@

class Web3ModuleTest:
def test_web3_client_version(self, w3: Web3) -> None:
time.sleep(1)
client_version = w3.client_version
self._check_web3_client_version(client_version)

Expand Down

0 comments on commit 21dc730

Please sign in to comment.