Skip to content

Conversation

@gmalouf
Copy link
Contributor

@gmalouf gmalouf commented Oct 4, 2025

Summary

Reworking min fee expectations in tests to consistently respect the consensus parameters.

Test Plan

Existing tests should all pass.

@gmalouf gmalouf self-assigned this Oct 4, 2025
@gmalouf gmalouf requested review from cce, Copilot and jannotti and removed request for Copilot and jannotti October 7, 2025 18:54
@algorand algorand deleted a comment from codecov bot Oct 7, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates test files to respect the consensus protocol's minimum fee parameter (MinTxnFee) instead of using hardcoded fee values. The changes ensure tests remain accurate across different consensus versions where minimum fees may vary.

  • Tests now dynamically retrieve and use the network's minimum fee from consensus parameters or suggested parameters API
  • Hardcoded fee values (commonly 1000 microAlgos) are replaced with protocol-aware fee calculations
  • Balance calculations and assertions are updated to account for variable fees

Reviewed Changes

Copilot reviewed 65 out of 65 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
test/scripts/e2e_subs/rest.sh Adds get_min_fee() function to retrieve network minimum fee via REST API
Multiple test shell scripts Replace hardcoded fees with dynamic MIN_FEE from get_min_fee()
Python test files Use goal.params().min_fee or params.MinFee instead of hardcoded values
Go test files Replace hardcoded fees with proto.MinTxnFee from consensus parameters
Expect test files Add GetSuggestedFee function and update fee handling
Config files Update default fee values and add placeholder MinTxnFee values for testing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@gmalouf gmalouf requested a review from jannotti October 8, 2025 20:20
@gmalouf gmalouf marked this pull request as ready for review October 8, 2025 20:20
@gmalouf gmalouf requested a review from algorandskiy October 10, 2025 16:23
jannotti
jannotti previously approved these changes Oct 10, 2025
Copy link
Contributor

@algorandskiy algorandskiy left a comment

Choose a reason for hiding this comment

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

Plus I still suggest to rewrite AlgorandGoal::GetSuggestedFee to use curl + jq instead of submitting a transaction

@codecov
Copy link

codecov bot commented Oct 17, 2025

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.97%. Comparing base (1625da7) to head (2978b2e).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
ledger/eval/eval.go 0.00% 2 Missing ⚠️
cmd/pingpong/runCmd.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6456      +/-   ##
==========================================
- Coverage   51.13%   50.97%   -0.17%     
==========================================
  Files         668      661       -7     
  Lines      112101   112005      -96     
==========================================
- Hits        57327    57094     -233     
- Misses      51907    52037     +130     
- Partials     2867     2874       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jannotti
jannotti previously approved these changes Oct 20, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 66 out of 66 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@gmalouf gmalouf merged commit 05c8fd1 into algorand:master Oct 21, 2025
39 checks passed
@gmalouf gmalouf deleted the min-fee-rework branch October 21, 2025 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants