fix: node bug error #75
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bumps [dtolnay/rust-toolchain](https://github.com/dtolnay/rust-toolchain) from d8352f6b1d2e870bc5716e7a6d9b65c4cc244a1a to 6d653acede28d24f02e3cd41383119e8b1b35921. - [Release notes](https://github.com/dtolnay/rust-toolchain/releases) - [Commits](dtolnay/rust-toolchain@d8352f6...6d653ac) --- updated-dependencies: - dependency-name: dtolnay/rust-toolchain dependency-version: 6d653acede28d24f02e3cd41383119e8b1b35921 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
update block cache with stale data
…ay/rust-toolchain-6d653acede28d24f02e3cd41383119e8b1b35921 Bump dtolnay/rust-toolchain from d8352f6b1d2e870bc5716e7a6d9b65c4cc244a1a to 6d653acede28d24f02e3cd41383119e8b1b35921
Refactor the sync assets script
Tag only release Docker builds as latest
npm audit fix
Co-authored-by: mononaut <83316221+mononaut@users.noreply.github.com>
Community/about page: replace Citadel with Nirvati
[accelerator] additional acceleration history API fixes
set content limit to 10mb
[accelerator] use /accelerator/accelerations/:txid in transaction component
Reviewer's GuideThis PR refactors the frontend sync-assets script into a promise‐based, modular workflow driven by a unified config object, introduces a new txid‐specific accelerator endpoint in the backend and updates the frontend to consume it with improved retry/filter logic, optimizes block indexing and caching via height cutoffs and skipMemoryCache support, raises express body size limits, enhances CI/CD workflows, and applies minor UI and integration fixes. Sequence diagram for new txid-specific acceleration data retrievalsequenceDiagram
participant TrackerComponent
participant ServicesApiService
participant BackendAPI
participant AccelerationRepository
TrackerComponent->>ServicesApiService: getAccelerationDataForTxid$(txid)
ServicesApiService->>BackendAPI: GET /accelerator/accelerations/:txid
BackendAPI->>AccelerationRepository: $getAccelerationInfoForTxid(txid)
AccelerationRepository-->>BackendAPI: Acceleration data or null
BackendAPI-->>ServicesApiService: Acceleration data or 404
ServicesApiService-->>TrackerComponent: Acceleration data or null
Note over TrackerComponent: Retry logic if no data and tx was accelerated
TrackerComponent->>TrackerComponent: Update accelerationInfo, setIsAccelerated()
Class diagram for AccelerationRepository and new methodclassDiagram
class AccelerationRepository {
+$getAccelerationInfoForTxid(txid: string): Promise<PublicAcceleration | null>
+$getAccelerationInfo(poolSlug: string | null, height: number | null, interval: string | null): Promise<PublicAcceleration[]>
...
}
Class diagram for ServicesApiService frontend changesclassDiagram
class ServicesApiService {
+getAccelerationDataForTxid$(txid: string): Observable<Acceleration>
+getAllAccelerationHistory$(params, limit?, findTxid?): Observable<Acceleration[]>
...
}
Class diagram for TrackerComponent acceleration logic changesclassDiagram
class TrackerComponent {
-fetchAcceleration$: Subject<number>
+accelerationInfo: Acceleration | null
+setIsAccelerated()
...
}
Class diagram for TransactionComponent acceleration logic changesclassDiagram
class TransactionComponent {
+accelerationInfo: Acceleration | null
+setIsAccelerated()
...
}
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Summary of ChangesHello @Dargon789, 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! This pull request introduces a new API endpoint for fetching specific transaction acceleration data, alongside significant improvements to block caching and indexing mechanisms in the backend. On the frontend, the way acceleration data is retrieved and displayed has been refactored for better reliability. Additionally, the asset synchronization script has been modernized, and a minor update to the 'About' page's community integrations has been made. These changes collectively aim to enhance the application's performance, data accuracy, and maintainability. Highlights
Ignored Files
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consolidate the duplicated retry and catchError RxJS logic in TransactionComponent and TrackerComponent into a shared operator or service to improve maintainability.
- Ensure that downloadLiquidAssets and other asynchronous calls in sync-assets.js are properly awaited or handled so errors aren’t silently ignored.
- Extract repeated configuration parsing (environment variables and JSON config) in sync-assets.js into reusable helper functions to reduce boilerplate and improve readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consolidate the duplicated retry and catchError RxJS logic in TransactionComponent and TrackerComponent into a shared operator or service to improve maintainability.
- Ensure that downloadLiquidAssets and other asynchronous calls in sync-assets.js are properly awaited or handled so errors aren’t silently ignored.
- Extract repeated configuration parsing (environment variables and JSON config) in sync-assets.js into reusable helper functions to reduce boilerplate and improve readability.
## Individual Comments
### Comment 1
<location> `backend/src/index.ts:145-147` </location>
<code_context>
res.setHeader('Access-Control-Expose-Headers', 'X-Total-Count,X-Mempool-Auth');
next();
})
- .use(express.urlencoded({ extended: true }))
- .use(express.text({ type: ['text/plain', 'application/base64'] }))
- .use(express.json())
+ .use(express.urlencoded({ extended: true, limit: '10mb' }))
+ .use(express.text({ type: ['text/plain', 'application/base64'], limit: '10mb' }))
+ .use(express.json({ limit: '10mb' }))
;
</code_context>
<issue_to_address>
**🚨 issue (security):** Request body size limit increased to 10mb, which may have security implications.
This change could increase vulnerability to denial-of-service attacks. Please ensure appropriate rate limiting and resource controls are in place.
</issue_to_address>
### Comment 2
<location> `frontend/sync-assets.js:226` </location>
<code_context>
+ return contents;
+};
+
+// Main: Download mining pool logos
+const downloadMiningPoolLogos = async () => {
+ console.log(`${LOG_TAG} \tChecking if mining pool logos needs downloading or updating...`);
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the three asset download functions into a single generic helper to eliminate duplication and simplify the code.
Here’s one way to collapse those three “downloadXYZ” functions into a single, easy-to-reuse helper and remove most of the duplication and extra indirection:
```js
// 1) New generic helper to sync a GitHub directory:
async function syncGithubDir({
repoPath, // e.g. '/repos/mempool/mining-pool-logos/contents/'
targetDir, // local directory to write into
cdnPattern, // { from: 'raw.githubusercontent.com/…', to: 'mempool.space/…' }
filter = item => item.type === 'file' && item.download_url,
itemName = item => item.name
}) {
ensureDirectory(targetDir);
const items = await fetchGitHubContents(repoPath);
const valid = items.filter(filter);
let downloaded = 0;
for (const it of valid) {
const name = itemName(it);
const filePath = path.join(targetDir, name);
const url = getCDNUrl(it.download_url, cdnPattern);
const did = await processFileItem(it.sha, url, filePath, targetDir, name);
if (did) downloaded++;
}
console.log(`${LOG_TAG} \tDownloaded ${downloaded}, skipped ${valid.length - downloaded}`);
}
```
```js
// 2) Replace the three functions + IIFE with just three calls:
(async () => {
if (config.verbose) console.log(`${LOG_TAG} Downloading mining pool logos`);
await syncGithubDir({
repoPath: '/repos/mempool/mining-pool-logos/contents/',
targetDir: `${ASSETS_PATH}/mining-pools/`,
cdnPattern: {
from: 'raw.githubusercontent.com/mempool/mining-pool-logos/master',
to: 'mempool.space/resources/mining-pools'
}
});
if (config.verbose) console.log(`${LOG_TAG} Downloading promo video subtitles`);
await syncGithubDir({
repoPath: '/repos/mempool/mempool-promo/contents/subtitles',
targetDir: `${ASSETS_PATH}/promo-video/`,
cdnPattern: {
from: 'raw.githubusercontent.com/mempool/mempool-promo/master/subtitles',
to: 'mempool.space/resources/promo-video'
}
});
if (config.verbose) console.log(`${LOG_TAG} Downloading promo video`);
await syncGithubDir({
repoPath: '/repos/mempool/mempool-promo/contents',
filter: it => it.name === 'promo.mp4',
itemName: () => 'mempool-promo.mp4',
targetDir: `${ASSETS_PATH}/promo-video/`,
cdnPattern: {
from: 'raw.githubusercontent.com/mempool/mempool-promo/master/promo.mp4',
to: 'mempool.space/resources/promo-video/mempool-promo.mp4'
}
});
console.log(`${LOG_TAG} Asset synchronization complete`);
})();
```
This shrinks ~150 lines of nearly-identical logic into one small helper plus three declarative calls, keeps all behavior intact, and removes deep nesting or duplicate promise chains.
</issue_to_address>
### Comment 3
<location> `frontend/src/app/components/tracker/tracker.component.ts:290` </location>
<code_context>
-
- this.accelerationInfo = acceleration;
- this.setIsAccelerated();
+ switchMap((blockHeight: number) => {
+ if (this.stateService.network === '' && this.stateService.env.ACCELERATOR && blockHeight >= 819500 ) {
+ return this.servicesApiService.getAccelerationDataForTxid$(this.txId).pipe(
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the block height check and retry logic into a helper function to simplify the observable pipeline.
You’ve correctly kept all existing behavior, but the nested switchMap/throwError/retry/catchError + the inline `if` is making the pipe very hard to follow. You can pull that block‐height check and retry strategy out into a small helper so the main stream reads like “filter → clear state → load → filter → subscribe”.
For example:
```ts
private loadAccelerationForTx(height: number): Observable<Acceleration|null> {
// only fetch on mainnet, post‐fork
if (this.stateService.network !== '' || height < 819500) {
return of(null);
}
return this.servicesApiService.getAccelerationDataForTxid$(this.txId).pipe(
// if we thought we should have data (this.tx.acceleration) but got none, retry up to 3×
switchMap(accData =>
accData
? of(accData)
: this.tx.acceleration
? throwError(() => 'retry')
: of(null)
),
retry({
count: 3,
delay: err =>
err === 'retry'
? timer(2000)
: throwError(() => err)
}),
catchError(() => of(null))
);
}
```
Then your subscription becomes:
```ts
this.fetchAccelerationSubscription =
this.fetchAcceleration$.pipe(
filter(() => this.stateService.env.ACCELERATOR),
tap(() => {
this.accelerationInfo = null;
this.setIsAccelerated();
}),
mergeMap(height => this.loadAccelerationForTx(height)),
filter((acc): acc is Acceleration => !!acc)
).subscribe(acceleration => {
// ...existing “if status===completed/failed” logic...
this.setIsAccelerated();
});
```
This:
1. Removes the big inline `if/else` + retry block
2. Keeps your service call + retry strategy exactly the same
3. Makes the pipe read “filter → reset → load → filter → subscribe” instead of nesting everything in one giant switchMap.
</issue_to_address>
### Comment 4
<location> `frontend/src/app/components/transaction/transaction.component.ts:345` </location>
<code_context>
-
- this.accelerationInfo = acceleration;
- this.setIsAccelerated();
+ switchMap((blockHeight: number) => {
+ if (this.stateService.network === '' && this.stateService.env.ACCELERATOR && blockHeight >= 819500 ) {
+ return this.servicesApiService.getAccelerationDataForTxid$(this.txId).pipe(
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the observable pipeline by moving network and height checks to early filters, handling missing data in the subscription, and extracting logic into helper functions.
```markdown
You can simplify by
1. Pushing your network + height check into early `filter` operators
2. Dropping the in-pipe `catchError` + `filter(Boolean)` and instead handling “no data” in your `subscribe` callback
3. (Optionally) extracting the service‐call and the status‐handling into small helpers
For example:
```ts
// 1. filter early, before the switchMap
this.fetchAccelerationSubscription = this.fetchAcceleration$.pipe(
filter(() => this.stateService.env.ACCELERATOR),
filter((height: number) =>
this.stateService.network === '' && height >= 819500
),
tap(() => {
this.accelerationInfo = null;
this.setIsAccelerated();
}),
// 2. keep only retry logic in‐pipe
switchMap(() =>
this.servicesApiService.getAccelerationDataForTxid$(this.txId).pipe(
switchMap(data =>
// retry only when we expected data but got none
this.tx.acceleration && !data
? throwError(() => 'retry')
: of(data)
),
retry({ count: 3, delay: 2000 })
)
)
).subscribe(acceleration => {
// handle “no data or giving up after retry”
if (!acceleration) {
this.waitingForAccelerationInfo = false;
this.setIsAccelerated();
return;
}
// existing completed/failed logic…
if (
(acceleration.status === 'completed' ||
acceleration.status === 'completed_provisional') &&
acceleration.pools.includes(acceleration.minedByPoolUniqueId)
) {
// …
}
if (
acceleration.status === 'failed' ||
acceleration.status === 'failed_provisional'
) {
// …
}
this.waitingForAccelerationInfo = false;
this.setIsAccelerated();
});
```
Optionally pull out the inner `switchMap` logic into something like
```ts
private getAccelerationStream(txId: string): Observable<Acceleration | null> { … }
private handleAcceleration(acc: Acceleration | null) { … }
```
to further declutter your main pipeline.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several features and fixes. A new backend endpoint is added to fetch acceleration data for a single transaction, with corresponding frontend changes to use this new endpoint, including improved retry logic. The backend indexing is tightened to skip stale blocks. The asset synchronization script has been significantly refactored to use modern async/await syntax, improving readability and maintainability.
My review includes two main points:
- In
acceleration.routes.ts, I've suggested improving the new endpoint handler by removing an unreachable code block and adding validation for thetxid. - In the refactored
sync-assets.jsscript, I've identified a potential for unhandled promise rejections and suggested a fix to make the script more robust.
Overall, the changes are well-structured and the refactoring of the sync script is a great improvement.
Dargon789
merged commit 7f99323
into
Dargon789:snyk-upgrade-7de335c821af1234699244ed64574edc
3 of 5 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Refactor asset synchronization script to use async/await with configurable CDN and dry run support, add API and UI support for fetching single transaction acceleration data with improved retry logic, tighten backend indexing to skip stale blocks, and update CI and contributor metadata accordingly.
New Features:
Bug Fixes:
Enhancements:
CI:
Documentation: