statetest: Reuse Transaction loading in TestMultiTransaction #556
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #556 +/- ##
==========================================
+ Coverage 97.03% 97.41% +0.37%
==========================================
Files 66 67 +1
Lines 6138 6183 +45
==========================================
+ Hits 5956 6023 +67
+ Misses 182 160 -22
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| EXPECT_EQ(tx.sender, 0xa0a1_address); | ||
| EXPECT_FALSE(tx.to.has_value()); | ||
| EXPECT_EQ(tx.max_gas_price, 0x7071); | ||
| EXPECT_EQ(tx.max_priority_gas_price, 0x7071); |
There was a problem hiding this comment.
Hm, are these correct? Are they both set to gasPrice? I thought the logic is more complex.
There was a problem hiding this comment.
It turns out you can you EIP-1559 logic this way also for legacy transactions.
There was a problem hiding this comment.
Yeah, the logic is as simple as that in https://eips.ethereum.org/EIPS/eip-1559:
def normalize_transaction(self, transaction: Transaction, signer_address: int) -> NormalizedTransaction:
# legacy transactions
if isinstance(transaction, TransactionLegacy):
return NormalizedTransaction(
signer_address = signer_address,
signer_nonce = transaction.signer_nonce,
gas_limit = transaction.gas_limit,
max_priority_fee_per_gas = transaction.gas_price,
max_fee_per_gas = transaction.gas_price,
destination = transaction.destination,
amount = transaction.amount,
payload = transaction.payload,
access_list = [],
)| ] | ||
| })"; | ||
|
|
||
| const auto tx = test::from_json<state::Transaction>(json::json::parse(input)); |
There was a problem hiding this comment.
Technically could add the checks for each of the zero values.
|
|
||
| using namespace evmone; | ||
|
|
||
| TEST(statetest_loader, tx_create_legacy) |
There was a problem hiding this comment.
Should probably add a test where both gasPrice and maxFeePerGas is present.
|
I'll squash some commits, once these changes are accepted. |
test/unittests/analysis_test.cpp
Outdated
| EXPECT_EQ(analysis.instrs[2].fn, op_tbl[OP_DUP1].fn); | ||
| EXPECT_EQ(analysis.instrs[8].fn, op_tbl[OP_POP].fn); | ||
| EXPECT_EQ(analysis.instrs[18].fn, op_tbl[OP_PUSH1].fn); | ||
| EXPECT_EQ(analysis.instrs[1].fn, op_tbl[evmone::OP_DUP2].fn); |
There was a problem hiding this comment.
Need to remove all these changes, needed locally in order to compile and managed to commit in.
|
Looks good to me, need to remove all |
Co-authored-by: Alex Beregszaszi <alex@rtfs.hu>
82d64ba to
18b83a3
Compare
| o.max_priority_gas_price = o.max_gas_price; | ||
| if (j.contains("maxFeePerGas") || j.contains("maxPriorityFeePerGas")) | ||
| throw std::invalid_argument( |
There was a problem hiding this comment.
This could be in braces {}, because multi-line.
No description provided.