Track fees for Chia Chains - #5454
PatelPrinci wants to merge 6 commits into
Conversation
|
The chia.ts adapter exports: |
bheluga
left a comment
bheluga
left a comment
There was a problem hiding this comment.
@PatelPrinci Thanks for the PR.
Its failing with 524 , probably website isnt functional/synced.
Any other alternative?
|
The chia.ts adapter exports: |
|
The chia.ts adapter exports: |
|
The chia.ts adapter exports: |
|
@bheluga here i haven't modified liquidity.ts then why it shows error? |
@PatelPrinci mostly because the commit which was merged by other contributors had ts errors, you need not worry about that. |
📝 WalkthroughWalkthroughThe PR introduces Chia blockchain support by adding a new fee data adapter that aggregates transaction fees from multiple external block data APIs, and extends the chain enum to include CHIA as a supported chain. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The chia.ts adapter exports: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@fees/chia.ts`:
- Around line 115-121: The adapter's declared start date ("start" on the adapter
object) is too old for the upstream APIs' ~14-day historical retention, causing
backfills to fail; update the adapter.start value in the adapter object (where
adapter, fetch, CHAIN.CHIA, and ProtocolType.CHAIN are defined) to a recent date
within the API retention window (e.g., within the last 14 days) or add a clear
comment/documentation next to the adapter.start field explaining the API
limitation and that only recent history is available so backfills beyond that
range will return empty/zero values.
- Around line 70-80: The code currently returns zero fees for dates older than
historicalLimit which masks missing data; replace the early-return block that
creates dailyFees (using historicalLimit, dailyFees, options.createBalances())
with throwing a descriptive Error (e.g., "Historical fee data unavailable for
YYYY-MM-DD: Chia APIs only expose ~14 days") so callers can handle/skip
backfills, and update the adapter start date (currently "2021-03-19") to a
realistic earliest-available date (or add adapter metadata documenting the
14-day limitation) to prevent attempts to backfill multi-year data.
- Around line 103-112: The code currently swallows API failures by logging and
returning zero fees (using results -> failedReasons and returning dailyFees from
options.createBalances() with addCGToken('chia', 0)); change this to throw an
error instead: build an informative Error (include failedReasons.join('; ') and
the dayStart date) and throw it so callers can handle retries/alerts rather than
receiving misleading zero fees; remove the return of dailyFees in the
all-APIs-failed branch.
🧹 Nitpick comments (3)
fees/chia.ts (3)
5-8: Remove unusedTransactioninterface.The
Transactioninterface is defined but never used in this file. The code usesany[]for blocks and accesses fields dynamically viaapiSource.timestampFieldandapiSource.feeField.🧹 Proposed fix
import { CHAIN } from "../helpers/chains"; import { httpGet } from "../utils/fetchURL"; -interface Transaction { - fee: string; - timestamp: number; -} - // Multiple API sources for Chia block data
82-101: Consider sequential fallback instead of parallel calls to all APIs.Currently, all 4 APIs are called simultaneously even though only one successful result is needed. This generates unnecessary load on external services. Consider using
Promise.any()or a sequential fallback pattern that stops after the first success.♻️ Proposed fix using sequential fallback
- const apiPromises = API_SOURCES.map(apiSource => - fetchFromApi(apiSource, dayStart, dayEnd).then(totalFeeXCH => ({ - apiSource, - totalFeeXCH - })) - ); - - const results = await Promise.allSettled(apiPromises); - - // Find the first fulfilled result - for (const result of results) { - if (result.status === 'fulfilled') { - const { apiSource, totalFeeXCH } = result.value; - console.log(`Successfully fetched fee data from ${apiSource.name}: ${totalFeeXCH} XCH`); - - const dailyFees = options.createBalances(); - dailyFees.addCGToken('chia', totalFeeXCH); - return { dailyFees }; - } - } + const errors: string[] = []; + + for (const apiSource of API_SOURCES) { + try { + const totalFeeXCH = await fetchFromApi(apiSource, dayStart, dayEnd); + console.log(`Successfully fetched fee data from ${apiSource.name}: ${totalFeeXCH} XCH`); + + const dailyFees = options.createBalances(); + dailyFees.addCGToken('chia', totalFeeXCH); + return { dailyFees }; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + errors.push(`${apiSource.name}: ${message}`); + console.log(`${apiSource.name} failed: ${message}`); + } + }
42-63: Add timeout to API requests and improve fee parsing.Two concerns:
No timeout:
httpGetcalls could hang indefinitely if an API is unresponsive. Consider adding a timeout option.Fee parsing:
parseInt(fee || 0)has issues:
- Missing radix parameter (should be
parseInt(fee, 10))- If
feeis already a number,parseIntis unnecessaryparseInt(0)returns0, butparseInt('0')also works, sofee || '0'would be clearerBlock limit: The 50-block limit may not capture all blocks in a day if block production is high. Consider whether pagination is needed.
♻️ Proposed fix for timeout and parsing
const fetchFromApi = async (apiSource: typeof API_SOURCES[0], dayStart: number, dayEnd: number): Promise<number> => { console.log(`Trying Chia API: ${apiSource.name}`); - const response = await httpGet(apiSource.url); + const response = await httpGet(apiSource.url, { timeout: 30000 }); if (!response || !response[apiSource.dataPath]) { throw new Error(`No ${apiSource.dataPath} data available from ${apiSource.name} API`); @@ const totalFeeMojos = dayBlocks.reduce((sum: number, block: any) => { const totalFeeMojos = dayBlocks.reduce((sum: number, block: any) => { const fee = block[apiSource.feeField]; - return sum + parseInt(fee || 0); + return sum + (typeof fee === 'number' ? fee : parseInt(fee || '0', 10)); }, 0);
| // API limitations for historical data | ||
| // Most Chia APIs only provide recent blocks (approximately last 10-14 days) | ||
| const historicalLimit = 14 * 24 * 60 * 60 * 1000; // 14 days in milliseconds | ||
|
|
||
| if (dayStart < now - historicalLimit) { | ||
| // For data older than 14 days, most APIs don't provide block data | ||
| console.log(`Historical fee data not available for ${new Date(dayStart).toISOString().slice(0, 10)} (API limitation - most Chia APIs only provide recent blocks)`); | ||
| const dailyFees = options.createBalances(); | ||
| dailyFees.addCGToken('chia', 0); | ||
| return { dailyFees }; | ||
| } |
There was a problem hiding this comment.
Historical data limitation should throw an error, not return 0.
Same concern as above — returning 0 fees for historical data masks the limitation and produces incorrect aggregate metrics. Consider either:
- Throwing an error indicating historical data is unavailable
- Documenting this limitation in the adapter metadata and having the system skip historical backfills
Additionally, the start date is set to "2021-03-19" (line 119), which will trigger backfill attempts for ~5 years of data that this adapter cannot provide.
🔧 Proposed fix — option 1: throw error
if (dayStart < now - historicalLimit) {
- // For data older than 14 days, most APIs don't provide block data
- console.log(`Historical fee data not available for ${new Date(dayStart).toISOString().slice(0, 10)} (API limitation - most Chia APIs only provide recent blocks)`);
- const dailyFees = options.createBalances();
- dailyFees.addCGToken('chia', 0);
- return { dailyFees };
+ throw new Error(`Historical fee data not available for ${new Date(dayStart).toISOString().slice(0, 10)} - Chia APIs only provide recent blocks (last ~14 days)`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // API limitations for historical data | |
| // Most Chia APIs only provide recent blocks (approximately last 10-14 days) | |
| const historicalLimit = 14 * 24 * 60 * 60 * 1000; // 14 days in milliseconds | |
| if (dayStart < now - historicalLimit) { | |
| // For data older than 14 days, most APIs don't provide block data | |
| console.log(`Historical fee data not available for ${new Date(dayStart).toISOString().slice(0, 10)} (API limitation - most Chia APIs only provide recent blocks)`); | |
| const dailyFees = options.createBalances(); | |
| dailyFees.addCGToken('chia', 0); | |
| return { dailyFees }; | |
| } | |
| // API limitations for historical data | |
| // Most Chia APIs only provide recent blocks (approximately last 10-14 days) | |
| const historicalLimit = 14 * 24 * 60 * 60 * 1000; // 14 days in milliseconds | |
| if (dayStart < now - historicalLimit) { | |
| throw new Error(`Historical fee data not available for ${new Date(dayStart).toISOString().slice(0, 10)} - Chia APIs only provide recent blocks (last ~14 days)`); | |
| } |
🤖 Prompt for AI Agents
In `@fees/chia.ts` around lines 70 - 80, The code currently returns zero fees for
dates older than historicalLimit which masks missing data; replace the
early-return block that creates dailyFees (using historicalLimit, dailyFees,
options.createBalances()) with throwing a descriptive Error (e.g., "Historical
fee data unavailable for YYYY-MM-DD: Chia APIs only expose ~14 days") so callers
can handle/skip backfills, and update the adapter start date (currently
"2021-03-19") to a realistic earliest-available date (or add adapter metadata
documenting the 14-day limitation) to prevent attempts to backfill multi-year
data.
| // All APIs failed - log the reasons | ||
| const failedReasons = results | ||
| .filter((result): result is PromiseRejectedResult => result.status === 'rejected') | ||
| .map(result => result.reason instanceof Error ? result.reason.message : String(result.reason)); | ||
|
|
||
| console.log(`All Chia APIs failed. Reasons:`, failedReasons.join('; ')); | ||
| console.log(`Returning 0 fees for ${new Date(dayStart).toISOString().slice(0, 10)} due to API unavailability`); | ||
| const dailyFees = options.createBalances(); | ||
| dailyFees.addCGToken('chia', 0); | ||
| return { dailyFees }; |
There was a problem hiding this comment.
Returning 0 fees on API failure masks data unavailability — throw an error instead.
Per reviewer feedback in the PR comments, "when existing APIs are unavailable, the preferred approach is to find new reliable data sources rather than simply catching the error and returning 0 fees. Returning 0 is not the best solution." Silently returning 0 produces misleading metrics and hides infrastructure issues.
The adapter should throw an error when all APIs fail, allowing the caller to handle it appropriately (retry, alert, etc.).
🔧 Proposed fix
// All APIs failed - log the reasons
const failedReasons = results
.filter((result): result is PromiseRejectedResult => result.status === 'rejected')
.map(result => result.reason instanceof Error ? result.reason.message : String(result.reason));
- console.log(`All Chia APIs failed. Reasons:`, failedReasons.join('; '));
- console.log(`Returning 0 fees for ${new Date(dayStart).toISOString().slice(0, 10)} due to API unavailability`);
- const dailyFees = options.createBalances();
- dailyFees.addCGToken('chia', 0);
- return { dailyFees };
+ throw new Error(`All Chia APIs failed. Reasons: ${failedReasons.join('; ')}`);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // All APIs failed - log the reasons | |
| const failedReasons = results | |
| .filter((result): result is PromiseRejectedResult => result.status === 'rejected') | |
| .map(result => result.reason instanceof Error ? result.reason.message : String(result.reason)); | |
| console.log(`All Chia APIs failed. Reasons:`, failedReasons.join('; ')); | |
| console.log(`Returning 0 fees for ${new Date(dayStart).toISOString().slice(0, 10)} due to API unavailability`); | |
| const dailyFees = options.createBalances(); | |
| dailyFees.addCGToken('chia', 0); | |
| return { dailyFees }; | |
| // All APIs failed - log the reasons | |
| const failedReasons = results | |
| .filter((result): result is PromiseRejectedResult => result.status === 'rejected') | |
| .map(result => result.reason instanceof Error ? result.reason.message : String(result.reason)); | |
| throw new Error(`All Chia APIs failed. Reasons: ${failedReasons.join('; ')}`); |
🤖 Prompt for AI Agents
In `@fees/chia.ts` around lines 103 - 112, The code currently swallows API
failures by logging and returning zero fees (using results -> failedReasons and
returning dailyFees from options.createBalances() with addCGToken('chia', 0));
change this to throw an error instead: build an informative Error (include
failedReasons.join('; ') and the dayStart date) and throw it so callers can
handle retries/alerts rather than receiving misleading zero fees; remove the
return of dailyFees in the all-APIs-failed branch.
| const adapter: Adapter = { | ||
| version: 1, | ||
| fetch, | ||
| chains: [CHAIN.CHIA], | ||
| start: "2021-03-19", // Chia mainnet launch date | ||
| protocolType: ProtocolType.CHAIN, | ||
| }; |
There was a problem hiding this comment.
LGTM on adapter structure, but start date conflicts with API limitations.
The adapter correctly uses CHAIN.CHIA and ProtocolType.CHAIN. However, the start date of "2021-03-19" spans ~5 years while the APIs only provide ~14 days of historical data. This will result in failed or zero-value backfills for most of the historical range. Consider setting start to a recent date or documenting this limitation clearly.
🤖 Prompt for AI Agents
In `@fees/chia.ts` around lines 115 - 121, The adapter's declared start date
("start" on the adapter object) is too old for the upstream APIs' ~14-day
historical retention, causing backfills to fail; update the adapter.start value
in the adapter object (where adapter, fetch, CHAIN.CHIA, and ProtocolType.CHAIN
are defined) to a recent date within the API retention window (e.g., within the
last 14 days) or add a clear comment/documentation next to the adapter.start
field explaining the API limitation and that only recent history is available so
backfills beyond that range will return empty/zero values.
|
closing as stale |
2 participants
Addresses issue: #2622
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.