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

Add integration test for rejected transactions because of insufficient balance #345

Merged
merged 5 commits into from
Dec 3, 2024

Conversation

facuMH
Copy link
Contributor

@facuMH facuMH commented Nov 26, 2024

This PR solves https://github.com/Fantom-foundation/sonic-admin/issues/84
A test it added that specifically verifies that transactions are rejected if the signing account does not have enough balance to pay for transferred value + gas_limit * max_fee_cap. This is tested for all 4 types of transactions.

The test uses a slice of Tx instead of a map because of the issue fixed in #346

@facuMH facuMH requested a review from LuisPH3 November 26, 2024 15:39
@facuMH facuMH self-assigned this Nov 26, 2024
@LuisPH3 LuisPH3 changed the title rejected tx test Add integration test for rejected transactions because of insuficcient balance. Nov 26, 2024
@facuMH facuMH force-pushed the facundo/rejected_tx_test branch from 538529c to 54d1cec Compare November 26, 2024 16:57
Copy link
Collaborator

@HerbertJordan HerbertJordan left a comment

Choose a reason for hiding this comment

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

Looks good, but I think transactions with Value != 0 should also be covered.

tests/rejected_tx_test.go Show resolved Hide resolved
tests/rejected_tx_test.go Outdated Show resolved Hide resolved
tests/rejected_tx_test.go Outdated Show resolved Hide resolved
tests/rejected_tx_test.go Show resolved Hide resolved
tests/rejected_tx_test.go Outdated Show resolved Hide resolved
tests/rejected_tx_test.go Show resolved Hide resolved
tests/rejected_tx_test.go Outdated Show resolved Hide resolved
@facuMH facuMH force-pushed the facundo/rejected_tx_test branch from 54d1cec to 7365e00 Compare November 27, 2024 08:49
tests/rejected_tx_test.go Outdated Show resolved Hide resolved
@facuMH facuMH requested a review from LuisPH3 November 27, 2024 09:06
@HerbertJordan HerbertJordan changed the title Add integration test for rejected transactions because of insuficcient balance. Add integration test for rejected transactions because of insufficient balance. Nov 28, 2024
@HerbertJordan HerbertJordan changed the title Add integration test for rejected transactions because of insufficient balance. Add integration test for rejected transactions because of insufficient balance Nov 28, 2024
Copy link
Collaborator

@HerbertJordan HerbertJordan left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@facuMH facuMH force-pushed the facundo/rejected_tx_test branch 3 times, most recently from 37fd189 to 46130d8 Compare December 2, 2024 10:23
@facuMH facuMH force-pushed the facundo/rejected_tx_test branch from 46130d8 to bf041e0 Compare December 3, 2024 08:36
@facuMH facuMH merged commit 18d9c82 into develop Dec 3, 2024
2 checks passed
@facuMH facuMH deleted the facundo/rejected_tx_test branch December 3, 2024 08:40
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.

3 participants