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.

Sign up for GitHub

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

Jump to bottom

Problem: tx inclusion logic when block gas limit exceeded is not tested #380

Merged
merged 1 commit into from
Mar 11, 2022
Merged

Problem: tx inclusion logic when block gas limit exceeded is not tested #380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions integration_tests/test_basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,3 +598,39 @@ def test_contract(cronos):
# call contract
greeter_call_result = contract.caller.greet()
assert "world" == greeter_call_result


def test_tx_inclusion(cronos):
"""
- send multiple heavy transactions at the same time.
- check they are included in consecutively blocks without failure.
"""
w3 = cronos.w3
# bigger than block_gas_limit/2, so at most one tx in a block
tx_gas_limit = 80000000
amount = 1000
# use different sender accounts to be able be send concurrently
signed_txs = []
for account in ["validator", "community", "signer1", "signer2"]:
signed_txs.append(
sign_transaction(
w3,
{
"to": ADDRS["validator"],
Copy link
Collaborator

@thomas-nguy thomas-nguy Mar 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

the actual gas to send one transaction is 21,000

From your test I understand that the anteHandler will count the gas_limit, not the actual gas used. Isnt it a problem? It means that one can potentially block other transactions by setting very high 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.

the actual gas to send one transaction is 21,000

From your test I understand that the anteHandler will count the gas_limit, not the actual gas used. Isnt it a problem?

Copy link
Collaborator Author

@yihuang yihuang Mar 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

yeah, but that's how tendermint/cosmos-sdk works right now, in check tx mode it doesn't actually run the transaction, so it can only rely on the gas limit specified by the user. it's to make the validator run more efficiently.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, I wonder if its possible to rely on the api estimate_gas to have a more accurate result... but it might bring performance degradation

"value": amount,
"gas": tx_gas_limit,
},
KEYS[account],
)
)

for signed in signed_txs:
w3.eth.send_raw_transaction(signed.rawTransaction)

receipts = [
w3.eth.wait_for_transaction_receipt(signed.hash) for signed in signed_txs
]

# the transactions should be included in differnt but consecutive blocks
for receipt, next_receipt in zip(receipts, receipts[1:]):
assert next_receipt.blockNumber == receipt.blockNumber + 1
2 changes: 1 addition & 1 deletion integration_tests/test_upgrade.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def post_init(path, base_port, config):
ini_path = path / chain_id / SUPERVISOR_CONFIG_FILE
ini = configparser.RawConfigParser()
ini.read_file(ini_path.open())
reg = re.compile(fr"^program:{chain_id}-node(\d+)")
reg = re.compile(rf"^program:{chain_id}-node(\d+)")
for section in ini.sections():
m = reg.match(section)
if m:
Expand Down