Skip to content

Refactor mcp-atp#11

Merged
GPSxtreme merged 1 commit into
mainfrom
refactoring
Jun 5, 2025
Merged

Refactor mcp-atp#11
GPSxtreme merged 1 commit into
mainfrom
refactoring

Conversation

@GPSxtreme
Copy link
Copy Markdown
Contributor

@GPSxtreme GPSxtreme commented Jun 5, 2025

Closes https://github.com/IQAIcom/issues/issues/300 when this PR merged

Changes

  • remove dev and prod abis and create a minimal abi instead
  • update swap service to reduce code
  • create config to maintain defaults with zod schema validation

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 5, 2025

🦋 Changeset detected

Latest commit: 4d45995

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@iqai/mcp-atp Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@GPSxtreme GPSxtreme merged commit e25243d into main Jun 5, 2025
1 check passed
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Hello @GPSxtreme, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello team,

Gemini here, providing a summary of this pull request authored by GPSxtreme. The primary goal of this PR is to refactor the mcp-atp package to improve its structure, configuration management, and simplify certain parts of the codebase, particularly the swap service.

The key changes involve introducing a centralized configuration module using dotenv and zod for environment variable validation, removing the distinction between development and production ABIs and API endpoints in favor of configurable values, and streamlining the SwapService logic. The README and tool parameter descriptions have also been updated to reflect these changes.

Highlights

  • Centralized Configuration: A new src/config.ts file is introduced using dotenv to load environment variables and zod for schema validation. This centralizes configuration logic and provides type safety and default values for critical settings like API URLs, router addresses, and token addresses.
  • Simplified ABIs and Environment Handling: The distinction between development and production ABIs and API endpoints (ATP_USE_DEV environment variable) has been removed. Instead, the necessary addresses and API URL are now configurable via environment variables (ATP_AGENT_ROUTER_ADDRESS, ATP_BASE_TOKEN_ADDRESS, ATP_API_URL) with defaults provided in the new config file. A minimal router ABI (ROUTER_MINIMAL_ABI) is introduced, containing only the buy and sell functions needed by the swap service.
  • Refactored Swap Service: The SwapService has been updated to use the new configuration module and the minimal ABI. The buy and sell methods are simplified, leveraging a new private approveTokenAllowance helper method to handle token approvals before executing the swap. The methods now consistently use the 3-argument buy/sell functions on the router contract.
  • Tool Parameter Updates: The buy and sell tools' amount parameter descriptions and validation regex have been updated to explicitly state that the amount should be provided as a string representing the value in wei units, rather than a standard decimal number.
  • Documentation and Dependency Updates: The README is updated to remove references to ATP_USE_DEV and correct the wallet private key environment variable name to ATP_WALLET_PRIVATE_KEY. The dotenv package is added as a dependency.

Changelog

Click here to see the changelog
  • .changeset/wise-trains-fix.md
    • Added a changeset entry documenting the updated router ABI, removal of the dev router ABI, and the new environment variable configuration options.
  • README.md
    • Removed documentation for the ATP_USE_DEV environment variable.
    • Corrected the required environment variable for wallet private key from WALLET_PRIVATE_KEY to ATP_WALLET_PRIVATE_KEY in the main description and example configurations.
  • package.json
    • Added dotenv dependency (^16.5.0).
  • pnpm-lock.yaml
    • Updated lock file to include dotenv dependency.
  • src/config.ts
    • New File: Created a configuration module using dotenv and zod.
    • Defines and validates environment variables: ATP_WALLET_PRIVATE_KEY, ATP_API_URL (with default), ATP_API_KEY, ATP_AGENT_ROUTER_ADDRESS (with default), and ATP_BASE_TOKEN_ADDRESS (with default).
    • Exports the parsed environment configuration (env) and default pagination constants (DEFAULT_PAGE, DEFAULT_LIMIT, MAX_LIMIT).
  • src/lib/router.abi.minimal.ts
    • New File: Created a minimal ABI (ROUTER_MINIMAL_ABI) containing only the buy and sell function definitions.
  • src/services/agent-logs.ts
    • Updated imports to use constants from ../config.js instead of ../constants.js.
    • Modified API endpoint construction to use env.ATP_API_URL instead of conditional logic based on process.env.ATP_USE_DEV.
  • src/services/agent-positions.ts
    • Updated imports to use configuration from ../config.js instead of ../constants.js.
    • Modified API endpoint construction to use env.ATP_API_URL instead of conditional logic based on process.env.ATP_USE_DEV.
  • src/services/agent-stats.ts
    • Updated imports to use configuration from ../config.js instead of ../constants.js.
    • Modified API endpoint construction to use env.ATP_API_URL instead of conditional logic based on process.env.ATP_USE_DEV.
  • src/services/get-agents.ts
    • Updated imports to use configuration from ../config.js instead of ../constants.js.
    • Modified API endpoint construction to use env.ATP_API_URL instead of conditional logic based on process.env.ATP_USE_DEV.
    • Updated the API endpoint path from /agents to /agents/top.
  • src/services/swap.ts
    • Removed imports for old constants and dev/prod ABIs.
    • Imported env from ../config.js and ROUTER_MINIMAL_ABI.
    • Updated constructor to initialize routerAddress, baseTokenAddress, and routerAbi using values from the env configuration.
    • Introduced a private approveTokenAllowance method to handle ERC20 approvals.
    • Refactored buy and sell methods to use the new approveTokenAllowance method and consistently call the router contract with the 3-argument buy/sell functions (including 0n for minAmountOut).
    • Removed conditional logic based on process.env.ATP_USE_DEV within the buy and sell methods.
  • src/tools/add-agent-log.ts
    • Imported env from ../config.js.
    • Updated to use env.ATP_API_KEY instead of process.env.ATP_API_KEY.
    • Formatted the success return message with line breaks for better readability.
  • src/tools/buy.ts
    • Imported env from ../config.js.
    • Updated the amount parameter schema regex (/^d+$/) and description to require a valid integer string representing the amount in wei.
    • Updated to use env.ATP_WALLET_PRIVATE_KEY instead of process.env.WALLET_PRIVATE_KEY.
    • Formatted the success return message with line breaks for better readability.
  • src/tools/get-agent-logs.ts
    • Updated imports to use constants from ../config.js instead of ../constants.js.
  • src/tools/get-agent-positions.ts
    • Removed import for zod.
    • Imported env from ../config.js.
    • Updated to use env.ATP_WALLET_PRIVATE_KEY instead of process.env.ATP_WALLET_PRIVATE_KEY.
  • src/tools/sell.ts
    • Imported env from ../config.js.
    • Updated the amount parameter schema regex (/^d+$/) and description to require a valid integer string representing the amount in wei.
    • Updated to use env.ATP_WALLET_PRIVATE_KEY instead of process.env.WALLET_PRIVATE_KEY.
    • Formatted the success return message with line breaks for better readability.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


Config files now clean,
Zod validates the env,
Swap code is streamlined.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request significantly refactors the mcp-atp package by centralizing configuration, simplifying contract ABIs, and streamlining the swap service. These are excellent improvements for maintainability and clarity.

The introduction of src/config.ts with Zod validation for environment variables is a standout improvement, making configuration robust. The move to a minimal ABI for router interactions is also a good step.

The primary concern identified is related to slippage protection in the SwapService, which should be addressed before merging. Other minor points are noted in the summary.

Summary of Findings

  • Changeset Clarity on amount Unit: In .changeset/wise-trains-fix.md, while ABI and env var changes are noted, it might be beneficial to explicitly mention that the amount parameter for buy/sell tools now expects units in wei, not ether. This is a significant change for users of the package. (Severity: Low, not commented due to settings)
  • Redundant Wallet Client Check: In src/services/swap.ts, the buy (lines 49-53) and sell (lines 79-83) methods include checks for walletClient and walletClient.account. Since approveTokenAllowance (which is called first) already performs this check, these subsequent checks might be considered redundant. However, they provide defense-in-depth. (Severity: Low, not commented due to settings)
  • Formatting of Return Strings in Tools: In src/tools/add-agent-log.ts (lines 52-55), src/tools/buy.ts (lines 51-54), and src/tools/sell.ts (lines 51-54), the success message strings are now multi-line template literals with leading/trailing newlines and indentation. This might affect the display if printed directly to a console or log. Consider using .trim() or adjusting the template literal for cleaner output if necessary. (Severity: Low, not commented due to settings)

Merge Readiness

The pull request introduces valuable refactorings and improvements. However, due to the high-severity concern regarding the lack of slippage protection in the SwapService (buy/sell operations), I recommend that these changes be addressed before merging. Addressing this will enhance the safety and reliability of token trading functionalities. I am unable to approve the pull request myself; please ensure further review and approval after addressing the feedback. Other minor points are listed in the findings summary for consideration.

Comment thread src/services/swap.ts
Comment thread src/services/swap.ts
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.

1 participant