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

♻️ Abstract SwapVenue Validation to a Method on the Type #62

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

Conversation

NotJeremyLiu
Copy link
Member

@NotJeremyLiu NotJeremyLiu commented Aug 8, 2023

This PR moves the SwapVenue validation logic out of the entry point contract's instantiate function and into a method implemented on the SwapVenue object itself.

@NotJeremyLiu NotJeremyLiu mentioned this pull request Aug 8, 2023
@NotJeremyLiu NotJeremyLiu force-pushed the jl/abstract-instantiation-validation branch from eca2c57 to 22ca57b Compare August 8, 2023 23:29
@@ -102,6 +103,25 @@ pub struct SwapVenue {
pub adapter_contract_address: String,
}

impl SwapVenue {
Copy link

Choose a reason for hiding this comment

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

I think this should be UncheckedSwapVenue

&self,
deps: &DepsMut,
swap_venue_map: Map<&str, Addr>,
) -> Result<Addr, SkipError> {
Copy link

Choose a reason for hiding this comment

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

This should return a SwapVenue

return Err(ContractError::DuplicateSwapVenueName);
}
// Validate the swap venue (checks for valid address and non-duplicate name)
let checked_swap_contract_address = swap_venue.validate(&deps, SWAP_VENUE_MAP)?;

// Store the swap venue name and contract address inside the swap venue map
SWAP_VENUE_MAP.save(
Copy link

Choose a reason for hiding this comment

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

Instead of an address, it should store a SwapVenue

@NotJeremyLiu NotJeremyLiu force-pushed the jl/abstract-instantiation-validation branch from 45aae8c to e27cdff Compare August 9, 2023 23:00
@NotJeremyLiu NotJeremyLiu marked this pull request as draft September 12, 2023 21:23
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.

2 participants