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

fix(rpc): estimate fee should throw an error if any txn fails #286

Merged
merged 6 commits into from
Oct 8, 2024

Conversation

apoorvsadana
Copy link
Contributor

@apoorvsadana apoorvsadana commented Sep 26, 2024

Currently, estimate_fee returns the estimate even if the txn is failing. This is incorrect and estimate_fee should fail with TxnExecutionError instead.

Pending: add test cases

@apoorvsadana apoorvsadana marked this pull request as draft September 26, 2024 14:19
@apoorvsadana apoorvsadana marked this pull request as ready for review October 3, 2024 09:12
@@ -46,8 +46,16 @@ pub async fn estimate_fee(

let execution_results = exec_context.re_execute_transactions([], transactions, true, validate)?;

let fee_estimates =
execution_results.iter().map(|result| exec_context.execution_result_to_fee_estimate(result)).collect();
let fee_estimates = execution_results.iter().enumerate().try_fold(Vec::new(), |mut acc, (index, result)| {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider using Vec::with_capacity to avoid unnecessary re-allocation since the max size is known.

@antiyro antiyro merged commit 945a721 into main Oct 8, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants