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

feat: mayan integration #322

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

kuchmenko
Copy link

No description provided.

Copy link
Collaborator

@0xh3rman 0xh3rman left a comment

Choose a reason for hiding this comment

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

Left some comments, you can address them later when you finish what you plan to do

crates/gem_evm/src/mayan/swift/deployment.rs Outdated Show resolved Hide resolved
crates/gem_evm/src/mayan/swift/swift.rs Outdated Show resolved Hide resolved
gemstone/Cargo.toml Outdated Show resolved Hide resolved
gemstone/src/swapper/mayan/fee_manager.rs Outdated Show resolved Hide resolved
gemstone/src/swapper/mayan/mayan_relayer.rs Outdated Show resolved Hide resolved
gemstone/src/swapper/mayan/mayan_relayer.rs Outdated Show resolved Hide resolved
gemstone/src/swapper/mayan/mayan_relayer.rs Outdated Show resolved Hide resolved
}

#[derive(Debug)]
pub struct PermitParams {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we can reuse gem_evm::erc2612.rs, change r/s to [u8; 32]

Copy link
Author

Choose a reason for hiding this comment

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

tbh I don't think we will use permit at all
I keep this structure just to keep the interface similar to what contract expects

gemstone/src/swapper/mayan/mayan_swift.rs Outdated Show resolved Hide resolved
gemstone/src/swapper/mayan/mayan_swift_provider.rs Outdated Show resolved Hide resolved
@kuchmenko kuchmenko marked this pull request as ready for review December 5, 2024 17:42
@kuchmenko kuchmenko requested a review from 0xh3rman December 5, 2024 17:43
@0xh3rman 0xh3rman requested a review from gemcoder21 December 5, 2024 23:42
Copy link
Collaborator

@0xh3rman 0xh3rman left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, lgtm overall, can you fix the lint too?

use primitives::Chain;

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum MayanSwiftDeploymentWormholeId {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick, this is really long, let's just call it WormholdChainId?

Some(evm_address.to_string())
);
assert_eq!(get_swift_deployment_by_chain(Chain::Polygon).map(|x| x.address), Some(evm_address.to_string()));
// assert_eq!(get_swift_deployment_by_chain(Chain::Avalanche).map(|x| x.address), Some(evm_address.to_string()));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// assert_eq!(get_swift_deployment_by_chain(Chain::Avalanche).map(|x| x.address), Some(evm_address.to_string()));

})?,
amountIn: amount_in,
permitParams: permit.map_or(MayanSwiftPermit::zero().to_contract_params(), |p| p.to_contract_params()),
swapProtocol: Address::from_str(swap_protocol).map_err(|e| MayanForwarderError::ABIError {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you add a From<AddressError> for MayanForwarderError, it would be easier to return an address error just passing .map_err(MayanForwarderError::from


#[derive(Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
#[allow(dead_code)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this can be removed now? all the allow(dead_code) in this file

params.from_chain.to_string()
};

let mut query_params = vec![
Copy link
Collaborator

Choose a reason for hiding this comment

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

small improvement: make this params as a struct, serde_urlencoded supports too

let chain = self.get_chain_by_wormhole_id(w_chain_id).ok_or(SwapperError::InvalidRoute)?;

if chain == Chain::Solana {
todo!()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you search and remove all todo!()?

})
}

fn convert_amount_to_wei(&self, amount: f64, decimals: u32) -> Result<String, SwapperError> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we move this to a common place to reuse later?

}

async fn fetch_quote_from_request(&self, request: &SwapQuoteRequest, provider: Arc<dyn AlienProvider>) -> Result<Quote, SwapperError> {
let asset_decimals = self.get_asset_decimals(request.from_asset.clone(), provider.clone()).await?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@gemcoder21 let's pass decimal in for assets? to save a network request, or we can add X-Cache-TTL for this request, wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's a good idea, I know it's used in other places too, like Thorchain. So pass Asset instead of AssetId in swap quote request?


async fn fetch_quote(&self, request: &SwapQuoteRequest, provider: Arc<dyn AlienProvider>) -> Result<SwapQuote, SwapperError> {
// Validate chain support
if !self.supported_chains().contains(&request.from_asset.chain) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we also check from chain != to chain?

}

async fn get_transaction_status(&self, chain: Chain, transaction_hash: &str, provider: Arc<dyn AlienProvider>) -> Result<bool, SwapperError> {
todo!();
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there any endpoint to query this tx status? we can add it later for this method

@gemwalletcom gemwalletcom deleted a comment Dec 14, 2024
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