-
Notifications
You must be signed in to change notification settings - Fork 518
refactor: return SourcifyChainMap from initializeSourcifyChains #2749
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
Changes from 6 commits
86bb246
cf3ca93
e128292
1564422
8404d7c
6fe9029
c16794e
cd59a6b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,10 +3,7 @@ import { resetDatabase } from "../helpers/helpers"; | |
| import type { ServerOptions } from "../../src/server/server"; | ||
| import { Server } from "../../src/server/server"; | ||
| import config from "config"; | ||
| import { | ||
| initializeSourcifyChains, | ||
| sourcifyChainsMap, | ||
| } from "../../src/sourcify-chains"; | ||
| import { initializeSourcifyChains } from "../../src/sourcify-chains"; | ||
| import type { StorageIdentifiers } from "../../src/server/services/storageServices/identifiers"; | ||
| import { RWStorageIdentifiers } from "../../src/server/services/storageServices/identifiers"; | ||
| import type { Pool } from "pg"; | ||
|
|
@@ -33,6 +30,8 @@ export class ServerFixture { | |
| readonly repositoryV1Path: string; | ||
| readonly testS3Path: string = testS3Path; | ||
| readonly testS3Bucket: string = testS3Bucket; | ||
| // Assigned in the before() hook below; safe to read inside it/beforeEach. | ||
| sourcifyChainsMap!: SourcifyChainMap; | ||
|
|
||
| private _server?: Server; | ||
|
|
||
|
|
@@ -65,7 +64,7 @@ export class ServerFixture { | |
| this.repositoryV1Path = config.get<string>("repositoryV1.path"); | ||
|
|
||
| before(async () => { | ||
| await initializeSourcifyChains(); | ||
| this.sourcifyChainsMap = await initializeSourcifyChains(); | ||
|
|
||
| process.env.SOURCIFY_POSTGRES_PORT = | ||
| process.env.DOCKER_HOST_POSTGRES_TEST_PORT || "5431"; | ||
|
|
@@ -84,7 +83,7 @@ export class ServerFixture { | |
| port: fixtureOptions_?.port || config.get<number>("server.port"), | ||
| maxFileSize: config.get<number>("server.maxFileSize"), | ||
| corsAllowedOrigins: config.get<string[]>("corsAllowedOrigins"), | ||
| chains: fixtureOptions_?.chains || sourcifyChainsMap, | ||
| chains: fixtureOptions_?.chains || this.sourcifyChainsMap, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not a problem of this PR, but a problem in general: below we always pass the I think in general it would be better to remove the sourcifyChainMap from the VerificationServiceOptions and instead pass a separate argument from the Server constructor to the VerificationService in order to avoid such mistakes.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think it should be in this PR? The change should be straightforward?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See c16794e about prioritizing Above comment for removing the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. doesn't have to be in this PR, but should be straight forward, yes. I would simply add the chain map as a separate argument to the VerificationService constructor. |
||
| solc: new SolcLocal(config.get("solcRepo"), config.get("solJsonRepo")), | ||
| vyper: new VyperLocal(config.get("vyperRepo")), | ||
| fe: new FeLocal(config.get("feRepo")), | ||
|
|
@@ -97,7 +96,7 @@ export class ServerFixture { | |
| this._server = new Server( | ||
| serverOptions, | ||
| { | ||
| sourcifyChainMap: sourcifyChainsMap, | ||
| sourcifyChainMap: this.sourcifyChainsMap, | ||
| solcRepoPath: config.get("solcRepo"), | ||
| solJsonRepoPath: config.get("solJsonRepo"), | ||
| vyperRepoPath: config.get("vyperRepo"), | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.