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 swap test #6060

Merged
merged 15 commits into from
Aug 29, 2024
Merged

fix swap test #6060

merged 15 commits into from
Aug 29, 2024

Conversation

estebanmino
Copy link
Contributor

@estebanmino estebanmino commented Aug 28, 2024

Fixes APP-####

using useConnectedToHardhatStore hook to fetch balances, so when you switch to hardhat all assets balances will update correctly

What changed (plus any additional context for devs)

Screen recordings / screenshots

What to test

@estebanmino estebanmino changed the title [WIP] fix swap test fix swap test Aug 28, 2024
@estebanmino estebanmino marked this pull request as ready for review August 28, 2024 22:41
@jinchung jinchung self-requested a review August 29, 2024 16:16
Comment on lines 112 to 113
console.log('testID', testID, 'aa');

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.log('testID', testID, 'aa');

@@ -79,10 +79,11 @@ export function valueBasedDecimalFormatter({
// Default to normal rounding if no rounding mode is specified
roundedAmount = divWorklet(roundWorklet(mulWorklet(amount, factor)), factor);
}
console.log('maximumDecimalPlaces', maximumDecimalPlaces);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.log('maximumDecimalPlaces', maximumDecimalPlaces);

@estebanmino estebanmino force-pushed the @esteban/fix-swap-test branch 2 times, most recently from 07fd741 to 580f8be Compare August 29, 2024 20:09
@estebanmino estebanmino force-pushed the @esteban/fix-swap-test branch from 580f8be to f6c4d96 Compare August 29, 2024 20:10
Copy link
Contributor

@walmat walmat left a comment

Choose a reason for hiding this comment

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

Tested detox locally once the provider fix was in. Passed so I'm going to optimistically approve this. Feel free to merge once CI passes and everything feels good. 💯

src/redux/gas.ts Outdated
})
)?.connection?.url;
const provider = getProvider({ chainId: ChainId.mainnet });
const providerUrl = provider?.connection?.url;
if (isHardHat(providerUrl)) {
Copy link
Member

Choose a reason for hiding this comment

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

we could replace this with the connectedToHardhat directly instead of dealing with providerUrl and the isHardHat check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep pushing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i went ahead and also removed all instances of isHardhat

@@ -1,6 +1,6 @@
import { Contract } from '@ethersproject/contracts';
import { keyBy, mapValues } from 'lodash';
import { web3Provider } from '@/handlers/web3'; // TODO JIN
Copy link
Member

Choose a reason for hiding this comment

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

❤️

Copy link
Member

@jinchung jinchung left a comment

Choose a reason for hiding this comment

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

🌮 🐐

@brunobar79 brunobar79 merged commit 63aafb9 into develop Aug 29, 2024
5 checks passed
@brunobar79 brunobar79 deleted the @esteban/fix-swap-test branch August 29, 2024 21:58
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.

4 participants