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

Feature/plugin coingecko #1761

Merged
merged 7 commits into from
Jan 9, 2025
Merged

Conversation

Lukapetro
Copy link
Contributor

@Lukapetro Lukapetro commented Jan 3, 2025

image

Risks

Low - This is an additive change that introduces a new plugin without modifying existing functionality. The main risks are:

  • Rate limiting from CoinGecko API
  • Potential timeout issues when the API is slow
  • Invalid cryptocurrency IDs causing failed requests

Background

What does this PR do?

  • Adds a new CoinGecko plugin for cryptocurrency price checking
  • Implements a GET_PRICE action that fetches current prices
  • Provides TypeScript types and interfaces for CoinGecko API responses
  • Includes environment validation for API configuration
  • Adds proper error handling for API calls

What kind of change is this?

Features (non-breaking change which adds functionality)

  • New plugin implementation
  • New API integration
  • New TypeScript types

Documentation changes needed?

My changes require a change to the project documentation:

  • Add CoinGecko plugin setup instructions
  • Document required/optional environment variables
  • Add usage examples for price checking
  • Update plugin list in main documentation

Testing

Where should a reviewer start?

  1. Review the plugin implementation in /plugin-coingecko/src/
  2. Check the main action implementation in getPrice.ts
  3. Verify environment validation in environment.ts
  4. Review type definitions in types.ts

Detailed testing steps

  1. Set up environment:

    COINGECKO_API_KEY=your_api_key
  2. Run agent locally and test natural language queries:

    • "What's the current price of Bitcoin?"
    • "Show me ETH price in EUR"
    • "How much is USDC worth?"
    • "Get the price of Solana"
  3. Verify error handling:

    • Try with invalid coin names
    • Test API timeout scenarios
    • Check rate limit handling

Future Expansion

  1. Extended CoinGecko Integration:

    • Support all coins listed on CoinGecko (currently ~10,000+ cryptocurrencies)
    • Add coin metadata (market cap, volume, supply)
    • Include historical price data and charts
    • Support multiple currency conversions in a single query
  2. Additional API Features:

    • /simple/supported_vs_currencies - Add support for all available fiat currencies
    • /coins/markets - Include detailed market data
    • /coins/{id}/tickers - Add exchange-specific price data
    • /coins/{id}/market_chart - Support price charts and historical analysis
    • /global - Add global crypto market metrics
    • /search - Implement fuzzy search for coin names
    • /exchange_rates - Support BTC-to-fiat conversion rates
    • /trending - Add trending coins information
  3. Enhanced Features:

    • Price alerts and notifications
    • Price comparison across multiple coins
    • Custom time range for historical data
    • Price change percentage tracking
    • Market sentiment analysis
    • Portfolio tracking capabilities

Discord username

0xspit

@ag-wnl
Copy link
Collaborator

ag-wnl commented Jan 5, 2025

Duplicate/Related: #1382

@sachiew
Copy link

sachiew commented Jan 6, 2025

Additional API Features:
/simple/supported_vs_currencies - Add support for all available fiat currencies
/coins/markets - Include detailed market data // can be used to obtain tokens of a specific category
/coins/{id}/tickers - Add exchange-specific price data
/coins/{id}/market_chart - Support price charts and historical analysis
/global - Add global crypto market metrics
/search - Implement fuzzy search for coin names
/exchange_rates - Support BTC-to-fiat conversion rates
/trending - Add trending coins information // useful to get trending categories and NFTs too

^These are good additions, would like to also suggest the following to expand some useful use cases:

  • /coins/{id} (docs) - query metadata like official websites, descriptions, categories, contract addresses (network availability) explorer links, social links etc. These help user to understand more about a specific coin, or can leverage the metadata for other complimentary use cases: social sentiments, category coin discovery.

Onchain DEX data (requires pro-api key authentication)

  • /onchain/networks/trending_pools (docs)- query trending pools/tokens on GeckoTerminal
  • /onchain/networks/../trending_pools (docs)- query trending pools/tokens of a specific network
  • /onchain/simple/networks/../token_price (docs) - query price of any token (4M+ tokens, including those not listed on CoinGecko).
  • /onchain/networks/../pools (docs) - query market data of any liquidity pools: price, liquidity, trading volume, etc (100+ network, 5M+ pools).

@Lukapetro
Copy link
Contributor Author

someone who works in the coingecko team contacted me on linkedin and enabled the pro API on my account so I could integrate the PRO version and test the endpoints. If we want to integrate this PR I can expand the endpoints as I suggested and @sachiew mentioned

@wtfsayo
Copy link
Member

wtfsayo commented Jan 7, 2025

someone who works in the coingecko team contacted me on linkedin and enabled the pro API on my account so I could integrate the PRO version and test the endpoints. If we want to integrate this PR I can expand the endpoints as I suggested and @sachiew mentioned

that's cool

@wtfsayo
Copy link
Member

wtfsayo commented Jan 7, 2025

#1808 was merged today; which is basic, you can work on this with more tailored functionality

@Lukapetro
Copy link
Contributor Author

since we have this plugin #1808, we can think about centralizing all similar services, like coingecko and coinmarketcap here, instead of having 2 separate plugins exposing different configurations.

In view of keeping the code clean and simple, at this point I would just remove the individual plugins, unifying them and using only this plugin ( which we can extend with the various features that the various coingecko and coinmarketcap offer )

let me know what you think @wtfsayo

@wtfsayo
Copy link
Member

wtfsayo commented Jan 8, 2025

since we have this plugin #1808, we can think about centralizing all similar services, like coingecko and coinmarketcap here, instead of having 2 separate plugins exposing different configurations.

In view of keeping the code clean and simple, at this point I would just remove the individual plugins, unifying them and using only this plugin ( which we can extend with the various features that the various coingecko and coinmarketcap offer )

let me know what you think @wtfsayo

I think I would keep individual plugins and make a common that is limited to basic pricing features and can be use other plugins as fallback!

Coingecko, CMC, DexScreener have different api capabilities afaik

@wtfsayo
Copy link
Member

wtfsayo commented Jan 8, 2025

Just make sure it uses same envs as common plugin #1808

@Lukapetro and same for your cmc plugin please; remove conflicts and I will merge

@Lukapetro
Copy link
Contributor Author

@wtfsayo in both coinmarketcap and coingecko plugin we are using the same APIs key as #1808:

  • COINMARKETCAP_API_KEY
  • COINGECKO_API_KEY

@wtfsayo wtfsayo enabled auto-merge January 9, 2025 09:12
@wtfsayo wtfsayo self-requested a review January 9, 2025 09:12
Copy link
Member

@wtfsayo wtfsayo left a comment

Choose a reason for hiding this comment

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

ok lets go

@wtfsayo wtfsayo merged commit 57f6754 into elizaOS:develop Jan 9, 2025
3 checks passed
@Lukapetro Lukapetro deleted the feature/plugin-coingecko branch January 9, 2025 09:18
@0xCardinalError
Copy link
Contributor

0xCardinalError commented Jan 12, 2025

Duplicate/Related: #1382

yeah it was duplicated, also when it was merged it was less functional then PR I was working on.
can you explain this @wtfsayo why was this PR that was started later than the mine chosen and merged and also why did @Lukapetro go to do this in the first place when coingecko was obviously worked on, tested, and reviewed in another PR.
I thought we were building this together and not against each other :(

@sachiew
Copy link

sachiew commented Jan 13, 2025

hey @0xCardinalError, this could be partly my mistake, as I communicated with both of you and may not have coordinated things well. Yes, agree we are definitely building together 💪 let's chat more in TG.

@ag-wnl
Copy link
Collaborator

ag-wnl commented Jan 13, 2025

Hey @0xCardinalError thanks for the contribution! as other reviewers state, it might have been an overlook, though I had initially marked your PR (which at the time had more discussion/progress it seemed).

With so many contributions coming in on the regular we reviewers sometimes have a tough time tracking progress across similar PRs. its definitely amazing how fast we are progressing, moving forward aim to improve communication between duplicate efforts more streamlined so no effort goes wasted.

Even if cases like this happen, if the other PR has features which can be added on top of the merged contribution, let's add them to the merged feature! and yes we all are in this together 💪

@0xCardinalError
Copy link
Contributor

Hey @0xCardinalError thanks for the contribution! as other reviewers state, it might have been an overlook, though I had initially marked your PR (which at the time had more discussion/progress it seemed).

With so many contributions coming in on the regular we reviewers sometimes have a tough time tracking progress across similar PRs. its definitely amazing how fast we are progressing, moving forward aim to improve communication between duplicate efforts more streamlined so no effort goes wasted.

Even if cases like this happen, if the other PR has features that can be added on top of the merged contribution, let's add them to the merged feature! and yes we all are in this together 💪

yeah, all I can do now is check if there is something else I have that is not already merged and add it. I would say I hope its not going to happen again but I already see it here #1865 (comment)
someone also started another plugin with a same/similar use case, with no regard to work already done.

If this continues it will become toxic really fast.

0xpi-ai pushed a commit to 0xpi-ai/NayariAI that referenced this pull request Jan 15, 2025
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.

6 participants