-
Notifications
You must be signed in to change notification settings - Fork 94
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
feat: unit test for receipts WithOtherFields #1464
base: main
Are you sure you want to change the base?
Conversation
c4eea63
to
60c54a6
Compare
…lu/kakarot-rpc into unit_test_WithOtherFields
tests/tests/eth_provider.rs
Outdated
.insert("run out of resources".to_string(), serde_json::Value::from("run out of resources")); | ||
|
||
// Insert the modified receipt into the database | ||
katana.eth_provider().database().upsert_transaction_receipt(receipt_with_other_fields.clone()).await.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally I would rather have a way to add these on the startup of the katana instance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not sure if I understood what you mean. Do you mean adding a isRunOutOfResources
receipt at Katana’s startup?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the point mentioned here by @greged93 is that it is a bit "useless" to add an upsert_transaction_receipt
function in the database implementation knowing that we only use this call for a test.
So what you could do is that when launching Katana, in the same way that we add logs and block headers, you add a single receipt with a run out of resources and this way it is already present in the database when you use the katana fixture, you don't need to insert it here.
Resolves: #1457
Pull Request type
Please check the type of change your PR introduces:
What is the new behavior?
Added test to verify WithOtherFields behavior in eth_provider.
Does this introduce a breaking change?