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

test(anvil): fix remaining TODOs/ignores, remove compiler dependency #7877

Merged
merged 2 commits into from
May 7, 2024

Conversation

DaniPopes
Copy link
Member

No description provided.

crates/anvil/tests/it/pubsub.rs Show resolved Hide resolved
// Sending the transaction is successful but reverts on chain.
let tx = provider.send_transaction(tx).await.unwrap();
let receipt = tx.get_receipt().await.unwrap();
assert!(!receipt.inner.inner.status());
Copy link
Member Author

@DaniPopes DaniPopes May 6, 2024

Choose a reason for hiding this comment

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

send / send_transaction fail with gas estimation layer because the node returns an RPC error if the call/estimation fails, whereas without the layer the transaction will be sent directly to the node, returning a receipt with "status": false.

This test illustrates the difference in where the error happens.

.await
.unwrap()
.into_stream()
.take(2)
.take(num_txs)
.fuse();
Copy link
Member Author

Choose a reason for hiding this comment

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

*full boolean params is not implemented in anvil

@@ -121,6 +121,9 @@ impl ProviderBuilder {
})
.wrap_err_with(|| format!("invalid provider URL: {url_str:?}"));

// Use the final URL string to guess if it's a local URL.
let is_local = url.as_ref().map_or(false, |url| guess_local_url(url.as_str()));
Copy link
Member Author

Choose a reason for hiding this comment

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

required to reduce polling interval for watch tests

to: Default::default(),
value: U256::from(0)
}
to: Address::ZERO,
Copy link
Member Author

Choose a reason for hiding this comment

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

This was already zero but I'm not sure if this is correct, as selfdestruct technically has a to address. cc @sealer3

Copy link
Contributor

@sealer3 sealer3 May 6, 2024

Choose a reason for hiding this comment

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

You're right - for selfdestruct operations, type, from, to, and value should all be given in the ots_getInternalOperations response, using the smart contract being self-destructed as the from address and the target of the selfdestruct as the to address.

@@ -153,6 +153,12 @@ pub fn transaction_request_to_typed(
}
}

fn has_optimism_fields(other: &OtherFields) -> bool {
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixes the optimism test, since it doesn't set the tx type explicitly

@DaniPopes DaniPopes requested a review from klkvr as a code owner May 6, 2024 19:47
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

nice

crates/anvil/tests/it/pubsub.rs Show resolved Hide resolved
@DaniPopes DaniPopes merged commit 0ed8726 into master May 7, 2024
20 checks passed
@DaniPopes DaniPopes deleted the dani/finish-anvil-tests branch May 7, 2024 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants