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.

Sign up for GitHub

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

Improve contract recompilation #475

Merged
merged 8 commits into from
Dec 6, 2024
Merged

Improve contract recompilation #475

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4517d45
Remove the check for contract deployment
Lbqds Nov 29, 2024
708c0cf
Update schema
Lbqds Nov 29, 2024
05ba0a7
Introduce skipRecompileIfDeployedOnMainnet flag
Lbqds Nov 29, 2024
d0aa6fb
Introduce skipRecompileContracts config
Lbqds Nov 29, 2024
bb531ff
Load mainnet node url from config
Lbqds Nov 29, 2024
d51b0bd
Rename and add more tests
Lbqds Nov 29, 2024
53828bf
Address comments
Lbqds Nov 29, 2024
35cb7d7
Update full node to 3.9.1
Lbqds Nov 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 13 additions & 14 deletions packages/cli/cli_internal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,21 @@
import { deployAndSaveProgress } from './scripts/deploy'
import { Configuration, DEFAULT_CONFIGURATION_VALUES } from './src/types'
import { createProject, genRalph } from './scripts/create-project'
import {
checkFullNodeVersion,
codegen,
getConfigFile,
getSdkFullNodeVersion,
isDeployed,
isNetworkLive,
loadConfig
} from './src'
import { checkFullNodeVersion, codegen, getConfigFile, getSdkFullNodeVersion, isNetworkLive, loadConfig } from './src'
import { Project } from './src/project'

function getConfig(options: any): Configuration {

Check warning on line 29 in packages/cli/cli_internal.ts

View workflow job for this annotation

GitHub Actions / build (20) 

Unexpected any. Specify a different type
const configFile = options.config ? (options.config as string) : getConfigFile()
console.log(`Loading alephium config file: ${configFile}`)
const config = loadConfig(configFile)
if (config.forceRecompile && config.skipRecompileIfDeployedOnMainnet) {
throw new Error(`The forceRecompile and skipRecompileIfDeployedOnMainnet flags cannot be enabled at the same time`)
}

if (config.forceRecompile && (config.skipRecompileContracts ?? []).length > 0) {
throw new Error(`The skipRecompileContracts cannot be specified when forceRecompile is enabled`)
}

const isDebugModeEnabled = config.enableDebugMode || options.debug
if (isDebugModeEnabled) enableDebugMode()
return { ...config, enableDebugMode: isDebugModeEnabled }
Expand Down Expand Up @@ -99,7 +99,7 @@
try {
const config = getConfig(options)
const networkId = checkAndGetNetworkId(options.network)
const nodeUrl = config.networks[networkId].nodeUrl

Check warning on line 102 in packages/cli/cli_internal.ts

View workflow job for this annotation

GitHub Actions / build (20) 

Generic Object Injection Sink
if (!(await isNetworkLive(nodeUrl))) {
throw new Error(`${networkId} is not live`)
}
Expand All @@ -111,17 +111,16 @@
console.log(`Full node version: ${connectedFullNodeVersion}`)

const cwd = path.resolve(process.cwd())
const isContractDeployed = isDeployed(config)
if (!config.forceRecompile && isContractDeployed) {
console.warn(`The contracts has been deployed on testnet/mainnet, and the artifacts will not be updated.`)
}
const project = await Project.compile(
config.compilerOptions,
cwd,
config.sourceDir,
config.artifactDir,
connectedFullNodeVersion,
config.forceRecompile || !isContractDeployed
config.forceRecompile,
config.skipRecompileIfDeployedOnMainnet,
config.skipRecompileContracts ?? [],
config.networks['mainnet'].nodeUrl
)
console.log('✅ Compilation completed!')
if (options.skipGenerate) {
Expand Down
9 changes: 5 additions & 4 deletions packages/cli/src/deployment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
getConfigFile,
getDeploymentFilePath,
getNetwork,
isDeployed,
loadConfig,
retryFetch,
taskIdToVariable,
Expand Down Expand Up @@ -168,7 +167,7 @@
}

getInstance<I extends ContractInstance>(
contract: ContractFactory<I, any>,

Check warning on line 170 in packages/cli/src/deployment.ts

View workflow job for this annotation

GitHub Actions / build (20) 

Unexpected any. Specify a different type
group?: number,
taskId?: string
): I | undefined {
Expand Down Expand Up @@ -219,7 +218,7 @@
return this.contracts.size === 0 && this.scripts.size === 0 && this.migrations.size === 0
}

marshal(): any {

Check warning on line 221 in packages/cli/src/deployment.ts

View workflow job for this annotation

GitHub Actions / build (20) 

Unexpected any. Specify a different type
return {
deployerAddress: this.deployerAddress,
contracts: Object.fromEntries(this.contracts),
Expand All @@ -228,7 +227,7 @@
}
}

static unmarshal(json: any): DeploymentsPerAddress {

Check warning on line 230 in packages/cli/src/deployment.ts

View workflow job for this annotation

GitHub Actions / build (20) 

Unexpected any. Specify a different type
const deployerAddress = json.deployerAddress as string
const contracts = new Map(Object.entries<DeployContractExecutionResult>(json.contracts))
const scripts = new Map(Object.entries<RunScriptResult>(json.scripts))
Expand Down Expand Up @@ -289,7 +288,7 @@
return true
}
// previous !== undefined if retry is false
return previous!.issueTokenAmount !== issueTokenAmount

Check warning on line 291 in packages/cli/src/deployment.ts

View workflow job for this annotation

GitHub Actions / build (20) 

Forbidden non-null assertion
}

async function needToRunScript(
Expand Down Expand Up @@ -596,15 +595,17 @@
const prevProjectArtifact = await ProjectArtifact.from(projectRootDir)
const artifactDir = configuration.artifactDir ?? DEFAULT_CONFIGURATION_VALUES.artifactDir
let project: Project | undefined = undefined
if (configuration.skipRecompile !== true) {
const forceRecompile = configuration.forceRecompile || !isDeployed(configuration)
if (configuration.skipRecompileOnDeployment !== true) {
project = await Project.compile(
configuration.compilerOptions,
path.resolve(process.cwd()),
configuration.sourceDir ?? DEFAULT_CONFIGURATION_VALUES.sourceDir,
artifactDir,
undefined,
forceRecompile
configuration.forceRecompile,
configuration.skipRecompileIfDeployedOnMainnet,
configuration.skipRecompileContracts ?? [],
configuration.networks['mainnet'].nodeUrl
)
}

Expand Down
143 changes: 116 additions & 27 deletions packages/cli/src/project.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,16 @@ import {
CompilerOptions,
Constant,
Enum,
TraceableError
TraceableError,
getContractCodeByCodeHash
} from '@alephium/web3'
import * as path from 'path'
import fs from 'fs'
import { promises as fsPromises } from 'fs'
import { parseError } from './error'

const crypto = new WebCrypto()
const defaultMainnetNodeUrl = 'https://node.mainnet.alephium.org'

class TypedMatcher<T extends SourceKind> {
matcher: RegExp
Expand Down Expand Up @@ -390,18 +392,18 @@ export class Project {
contracts: Map<string, Compiled<Contract>>,
scripts: Map<string, Compiled<Script>>,
globalWarnings: string[],
changedSources: string[],
changedContracts: string[],
forceRecompile: boolean,
errorOnWarnings: boolean
): void {
const warnings: string[] = forceRecompile ? globalWarnings : []
contracts.forEach((contract) => {
if (Project.needToUpdate(forceRecompile, changedSources, contract.sourceInfo.name)) {
if (Project.needToUpdate(forceRecompile, changedContracts, contract.sourceInfo.name)) {
warnings.push(...contract.warnings)
}
})
scripts.forEach((script) => {
if (Project.needToUpdate(forceRecompile, changedSources, script.sourceInfo.name)) {
if (Project.needToUpdate(forceRecompile, changedContracts, script.sourceInfo.name)) {
warnings.push(...script.warnings)
}
})
Expand Down Expand Up @@ -475,8 +477,8 @@ export class Project {
return fsPromises.writeFile(filePath, JSON.stringify(object, null, 2))
}

private static needToUpdate(forceRecompile: boolean, changedSources: string[], name: string): boolean {
return forceRecompile || changedSources.includes(name)
private static needToUpdate(forceRecompile: boolean, changedContracts: string[], name: string): boolean {
return forceRecompile || changedContracts.includes(name)
}

private async checkMethodIndex(newArtifact: Compiled<Contract>) {
Expand All @@ -500,7 +502,7 @@ export class Project {
private async saveArtifactsToFile(
projectRootDir: string,
forceRecompile: boolean,
changedSources: string[]
changedContracts: string[]
): Promise<void> {
const artifactsRootDir = this.artifactsRootDir
const saveToFile = async function (compiled: Compiled<Artifact>): Promise<void> {
Expand All @@ -512,29 +514,29 @@ export class Project {
return fsPromises.writeFile(artifactPath, compiled.artifact.toString())
}
for (const [_, contract] of this.contracts) {
if (Project.needToUpdate(forceRecompile, changedSources, contract.sourceInfo.name)) {
if (Project.needToUpdate(forceRecompile, changedContracts, contract.sourceInfo.name)) {
await saveToFile(contract)
} else {
await this.checkMethodIndex(contract)
}
}
for (const [_, script] of this.scripts) {
if (Project.needToUpdate(forceRecompile, changedSources, script.sourceInfo.name)) {
if (Project.needToUpdate(forceRecompile, changedContracts, script.sourceInfo.name)) {
await saveToFile(script)
}
}
await this.saveStructsToFile()
await this.saveConstantsToFile()
await this.saveProjectArtifact(projectRootDir, forceRecompile, changedSources)
await this.saveProjectArtifact(projectRootDir, forceRecompile, changedContracts)
}

private async saveProjectArtifact(projectRootDir: string, forceRecompile: boolean, changedSources: string[]) {
private async saveProjectArtifact(projectRootDir: string, forceRecompile: boolean, changedContracts: string[]) {
if (!forceRecompile) {
// we should not update the `codeHashDebug` if the `forceRecompile` is disable
const prevProjectArtifact = await ProjectArtifact.from(projectRootDir)
if (prevProjectArtifact !== undefined) {
for (const [name, info] of this.projectArtifact.infos) {
if (!changedSources.includes(name)) {
if (!changedContracts.includes(name)) {
const prevInfo = prevProjectArtifact.infos.get(name)
info.bytecodeDebugPatch = prevInfo?.bytecodeDebugPatch ?? info.bytecodeDebugPatch
info.codeHashDebug = prevInfo?.codeHashDebug ?? info.codeHashDebug
Expand Down Expand Up @@ -588,6 +590,67 @@ export class Project {
}
}

private static async getDeployedContracts(
mainnetNodeUrl: string | undefined,
sourceInfos: SourceInfo[],
artifactsRootDir: string
): Promise<string[]> {
const nodeProvider = new NodeProvider(mainnetNodeUrl ?? defaultMainnetNodeUrl)
Copy link
Member

Choose a reason for hiding this comment

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

Let's check NetworkID to ensure that it's mainnet. It might be other networks by accident.

Copy link
Member

Choose a reason for hiding this comment

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

It seems mainetNodeUrl won't be undefined, so this check mainnetNodeUrl ?? defaultMainnetNodeUrl may not work.

Copy link
Member Author

Choose a reason for hiding this comment

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

We support both alephium.config.ts and alephium.config.js configurations. When using alephium.config.ts, the compiler will ensure that the node url is not undefined. But when using alephium.config.js, the node url may be undefined.

Copy link
Member

Choose a reason for hiding this comment

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

okay

const result: string[] = []
for (const sourceInfo of sourceInfos) {
const artifactPath = sourceInfo.getArtifactPath(artifactsRootDir)
if (sourceInfo.type === SourceKind.Contract) {
try {
const content = await fsPromises.readFile(artifactPath)
const artifact = JSON.parse(content.toString())
const codeHash = artifact['codeHash']
const contractCode = await getContractCodeByCodeHash(nodeProvider, codeHash)
if (contractCode !== undefined) result.push(artifact.name)
} catch (error) {
console.error(`Failed to check the contract deployment: ${sourceInfo.name}, error: ${error}`)
}
}
}
return result
}

private static async filterChangedContracts(
mainnetNodeUrl: string | undefined,
sourceInfos: SourceInfo[],
artifactsRootDir: string,
changedContracts: string[],
skipRecompileIfDeployedOnMainnet: boolean,
skipRecompileContracts: string[]
): Promise<string[]> {
let deployedContracts: string[]
if (skipRecompileIfDeployedOnMainnet) {
const remainSourceInfo = sourceInfos.filter(
(s) => s.type === SourceKind.Contract && !skipRecompileContracts.includes(s.name)
)
deployedContracts = await this.getDeployedContracts(mainnetNodeUrl, remainSourceInfo, artifactsRootDir)
} else {
deployedContracts = []
}

const filteredChangedContracts: string[] = []
changedContracts.forEach((c) => {
if (skipRecompileContracts.includes(c)) {
console.warn(
`The contract ${c} is in the skipRecompileContracts list. Even if the contract is updated, the code will not be regenerated. ` +
`To regenerate the bytecode, please remove the contract from the skipRecompileContracts list.`
)
} else if (deployedContracts.includes(c)) {
console.warn(
`The contract ${c} has already been deployed to the mainnet. Even if the contract is updated, the bytecode will not be regenerated. ` +
`To regenerate the bytecode, please enable the forceCompile flag.`
)
} else {
filteredChangedContracts.push(c)
}
})
return filteredChangedContracts
}

private static async compile_(
fullNodeVersion: string,
provider: NodeProvider,
Expand All @@ -597,8 +660,11 @@ export class Project {
artifactsRootDir: string,
errorOnWarnings: boolean,
compilerOptions: node.CompilerOptions,
changedSources: string[],
forceRecompile: boolean
changedContracts: string[],
forceRecompile: boolean,
skipRecompileIfDeployedOnMainnet: boolean,
skipRecompileContracts: string[],
mainnetNodeUrl: string | undefined
): Promise<Project> {
const removeDuplicates = sourceInfos.reduce((acc: SourceInfo[], sourceInfo: SourceInfo) => {
if (acc.find((info) => info.sourceCodeHash === sourceInfo.sourceCodeHash) === undefined) {
Expand Down Expand Up @@ -644,7 +710,7 @@ export class Project {
contracts,
scripts,
result.warnings ?? [],
changedSources,
changedContracts,
forceRecompile,
errorOnWarnings
)
Expand All @@ -659,7 +725,15 @@ export class Project {
result.enums ?? [],
projectArtifact
)
await project.saveArtifactsToFile(projectRootDir, forceRecompile, changedSources)
const filteredChangedContracts = await Project.filterChangedContracts(
mainnetNodeUrl,
sourceInfos,
artifactsRootDir,
changedContracts,
skipRecompileIfDeployedOnMainnet,
skipRecompileContracts
)
await project.saveArtifactsToFile(projectRootDir, forceRecompile, filteredChangedContracts)
return project
}

Expand All @@ -671,8 +745,11 @@ export class Project {
artifactsRootDir: string,
errorOnWarnings: boolean,
compilerOptions: node.CompilerOptions,
changedSources: string[],
forceRecompile: boolean
changedContracts: string[],
forceRecompile: boolean,
skipRecompileIfDeployedOnMainnet: boolean,
skipRecompileContracts: string[],
mainnetNodeUrl: string | undefined
): Promise<Project> {
const projectArtifact = await ProjectArtifact.from(projectRootDir)
if (projectArtifact === undefined) {
Expand Down Expand Up @@ -725,8 +802,11 @@ export class Project {
artifactsRootDir,
errorOnWarnings,
compilerOptions,
changedSources,
forceRecompile
changedContracts,
forceRecompile,
skipRecompileIfDeployedOnMainnet,
skipRecompileContracts,
mainnetNodeUrl
)
}
}
Expand Down Expand Up @@ -856,19 +936,22 @@ export class Project {
contractsRootDir = Project.DEFAULT_CONTRACTS_DIR,
artifactsRootDir = Project.DEFAULT_ARTIFACTS_DIR,
defaultFullNodeVersion: string | undefined = undefined,
forceRecompile = false
forceRecompile = false,
skipRecompileIfDeployedOnMainnet = false,
skipRecompileContracts: string[] = [],
mainnetNodeUrl: string | undefined = undefined
): Promise<Project> {
const provider = web3.getCurrentNodeProvider()
const fullNodeVersion = defaultFullNodeVersion ?? (await provider.infos.getInfosVersion()).version
const sourceFiles = await Project.loadSourceFiles(projectRootDir, contractsRootDir)
const { errorOnWarnings, ...nodeCompilerOptions } = { ...DEFAULT_COMPILER_OPTIONS, ...compilerOptionsPartial }
const projectArtifact = await ProjectArtifact.from(projectRootDir)
const changedSources = projectArtifact?.getChangedSources(sourceFiles) ?? sourceFiles.map((s) => s.name)
const changedContracts = projectArtifact?.getChangedSources(sourceFiles) ?? sourceFiles.map((s) => s.name)
if (
forceRecompile ||
projectArtifact === undefined ||
projectArtifact.needToReCompile(nodeCompilerOptions, fullNodeVersion) ||
changedSources.length > 0
changedContracts.length > 0
) {
if (fs.existsSync(artifactsRootDir)) {
removeOldArtifacts(artifactsRootDir, sourceFiles)
Expand All @@ -883,8 +966,11 @@ export class Project {
artifactsRootDir,
errorOnWarnings,
nodeCompilerOptions,
changedSources,
forceRecompile
changedContracts,
forceRecompile,
skipRecompileIfDeployedOnMainnet,
skipRecompileContracts,
mainnetNodeUrl
)
}
// we need to reload those contracts that did not regenerate bytecode
Expand All @@ -896,8 +982,11 @@ export class Project {
artifactsRootDir,
errorOnWarnings,
nodeCompilerOptions,
changedSources,
forceRecompile
changedContracts,
forceRecompile,
skipRecompileIfDeployedOnMainnet,
skipRecompileContracts,
mainnetNodeUrl
)
}
}
4 changes: 3 additions & 1 deletion packages/cli/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,14 @@ export interface Configuration<Settings = unknown> {
deploymentScriptDir?: string
deploymentsDir?: string
compilerOptions?: CompilerOptions
skipRecompile?: boolean
skipRecompileOnDeployment?: boolean

networks: Record<NetworkId, Network<Settings>>

enableDebugMode?: boolean
forceRecompile?: boolean
skipRecompileIfDeployedOnMainnet?: boolean
skipRecompileContracts?: string[]
}

export const DEFAULT_CONFIGURATION_VALUES = {
Copy link
Member

Choose a reason for hiding this comment

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

Should we enable skipRecompileOnDeployment by default?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm a bit hesitant about this because most of the contract compilation occurs during the development phase. Once the contract is deployed to the mainnet, the number of compilations should be much less.

If it's enabled by default, unnecessary checks could also be made on the mainnet during the development phase.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. Let's not enable it by default.

Expand Down
Loading
Loading