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: Improve tests of exchange_router contract. #555

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

khaeljy
Copy link
Contributor

@khaeljy khaeljy commented Oct 23, 2023

Pull Request type

Please add the labels corresponding to the type of changes your PR introduces:

  • Testing

What is the current behavior?

Resolves: #544

What is the new behavior?

Does this introduce a breaking change?

NO

Other information

@khaeljy khaeljy marked this pull request as draft October 23, 2023 17:36
@@ -288,13 +288,13 @@ mod ExchangeRouter {

fn create_deposit(ref self: ContractState, params: CreateDepositParams) -> felt252 {
let data_store = self.data_store.read();
global_reentrancy_guard::non_reentrant_before(data_store);
// global_reentrancy_guard::non_reentrant_before(data_store);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The global_reentrancy_guard::non_reentrant_before function is also called in the create_deposit function.
So I think there's one call too many, please, can you check if it's really this call that should be removed and not the one from the create_deposit function?


key
}

fn cancel_deposit(ref self: ContractState, key: felt252) {
let data_store = self.data_store.read();
global_reentrancy_guard::non_reentrant_before(data_store);
// global_reentrancy_guard::non_reentrant_before(data_store);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The global_reentrancy_guard::non_reentrant_before function is also called in the cancel_deposit function.
So I think there's one call too many, please, can you check if it's really this call that should be removed and not the one from the cancel_deposit function?

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.

test: Improve tests of exchange_router contract.
1 participant