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

Improve contract recompilation #475

Merged
merged 8 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
9 changes: 8 additions & 1 deletion packages/cli/cli_internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ program
throw new Error(`${networkId} is not live`)
}

if (config.forceRecompile && config.skipRecompileIfDeployedOnMainnet) {
throw new Error(
`The forceRecompile and skipRecompileIfDeployedOnMainnet flags cannot be enabled at the same time`
)
}

web3.setCurrentNodeProvider(nodeUrl)
const connectedFullNodeVersion = (await web3.getCurrentNodeProvider().infos.getInfosVersion()).version
const sdkFullNodeVersion = getSdkFullNodeVersion()
Expand All @@ -109,7 +115,8 @@ program
config.sourceDir,
config.artifactDir,
connectedFullNodeVersion,
config.forceRecompile
config.forceRecompile,
config.skipRecompileIfDeployedOnMainnet
)
console.log('✅ Compilation completed!')
if (options.skipGenerate) {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ export async function deploy<Settings = unknown>(
const prevProjectArtifact = await ProjectArtifact.from(projectRootDir)
const artifactDir = configuration.artifactDir ?? DEFAULT_CONFIGURATION_VALUES.artifactDir
let project: Project | undefined = undefined
if (configuration.skipRecompile !== true) {
if (configuration.skipRecompileOnDeployment !== true) {
project = await Project.compile(
configuration.compilerOptions,
path.resolve(process.cwd()),
Expand Down
110 changes: 83 additions & 27 deletions packages/cli/src/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,16 @@ import {
CompilerOptions,
Constant,
Enum,
TraceableError
TraceableError,
getContractByCodeHash
} 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 mainnetNodeUrl = '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,48 @@ export class Project {
}
}

private static async getDeployedContracts(sourceInfos: SourceInfo[], artifactsRootDir: string): Promise<string[]> {
const nodeProvider = new NodeProvider(mainnetNodeUrl)
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 getContractByCodeHash(nodeProvider, codeHash)
if (contractCode !== undefined) result.push(artifact.name)
} catch (error) {
console.error(`Failed to load contract artifact: ${sourceInfo.name}`)
}
}
}
return result
}

private static async filterChangedContracts(
sourceInfos: SourceInfo[],
artifactsRootDir: string,
changedContracts: string[],
skipRecompileIfDeployedOnMainnet: boolean
): Promise<string[]> {
if (!skipRecompileIfDeployedOnMainnet) return changedContracts
const deployedContracts = await this.getDeployedContracts(sourceInfos, artifactsRootDir)
const filteredChangedContracts: string[] = []
changedContracts.forEach((c) => {
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 +641,9 @@ export class Project {
artifactsRootDir: string,
errorOnWarnings: boolean,
compilerOptions: node.CompilerOptions,
changedSources: string[],
forceRecompile: boolean
changedContracts: string[],
forceRecompile: boolean,
skipRecompileIfDeployedOnMainnet: boolean
): 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 +689,7 @@ export class Project {
contracts,
scripts,
result.warnings ?? [],
changedSources,
changedContracts,
forceRecompile,
errorOnWarnings
)
Expand All @@ -659,7 +704,13 @@ export class Project {
result.enums ?? [],
projectArtifact
)
await project.saveArtifactsToFile(projectRootDir, forceRecompile, changedSources)
const filteredChangedContracts = await Project.filterChangedContracts(
sourceInfos,
artifactsRootDir,
changedContracts,
skipRecompileIfDeployedOnMainnet
)
await project.saveArtifactsToFile(projectRootDir, forceRecompile, filteredChangedContracts)
return project
}

Expand All @@ -671,8 +722,9 @@ export class Project {
artifactsRootDir: string,
errorOnWarnings: boolean,
compilerOptions: node.CompilerOptions,
changedSources: string[],
forceRecompile: boolean
changedContracts: string[],
forceRecompile: boolean,
skipRecompileIfDeployedOnMainnet: boolean
): Promise<Project> {
const projectArtifact = await ProjectArtifact.from(projectRootDir)
if (projectArtifact === undefined) {
Expand Down Expand Up @@ -725,8 +777,9 @@ export class Project {
artifactsRootDir,
errorOnWarnings,
compilerOptions,
changedSources,
forceRecompile
changedContracts,
forceRecompile,
skipRecompileIfDeployedOnMainnet
)
}
}
Expand Down Expand Up @@ -856,19 +909,20 @@ export class Project {
contractsRootDir = Project.DEFAULT_CONTRACTS_DIR,
artifactsRootDir = Project.DEFAULT_ARTIFACTS_DIR,
defaultFullNodeVersion: string | undefined = undefined,
forceRecompile = false
forceRecompile = false,
skipRecompileIfDeployedOnMainnet = false
): 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 +937,9 @@ export class Project {
artifactsRootDir,
errorOnWarnings,
nodeCompilerOptions,
changedSources,
forceRecompile
changedContracts,
forceRecompile,
skipRecompileIfDeployedOnMainnet
)
}
// we need to reload those contracts that did not regenerate bytecode
Expand All @@ -896,8 +951,9 @@ export class Project {
artifactsRootDir,
errorOnWarnings,
nodeCompilerOptions,
changedSources,
forceRecompile
changedContracts,
forceRecompile,
skipRecompileIfDeployedOnMainnet
)
}
}
3 changes: 2 additions & 1 deletion packages/cli/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,13 @@ 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
}

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
17 changes: 17 additions & 0 deletions packages/web3/src/contract/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2159,3 +2159,20 @@ export const getContractIdFromUnsignedTx = async (

// This function only works in the simple case where a single non-subcontract is created in the tx
export const getTokenIdFromUnsignedTx = getContractIdFromUnsignedTx

export async function getContractByCodeHash(
nodeProvider: NodeProvider,
codeHash: HexString
): Promise<HexString | undefined> {
Copy link
Member

Choose a reason for hiding this comment

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

We need to consider 429 errors due to rate limiting, and retry a few times if 429 hits

Copy link
Member

Choose a reason for hiding this comment

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

@Lbqds It seems this one is not resolved yet?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was resolved in the last commit. Since we don’t want to introduce a retry mechanism in web3, the retry on failure will be done in the cli.

if (isHexString(codeHash) && codeHash.length === 64) {
try {
return await nodeProvider.contracts.getContractsCodeHashCode(codeHash)
} catch (error) {
if (error instanceof Error && error.message.includes('not found')) {
return undefined
}
throw new TraceableError(`Failed to get contract by code hash ${codeHash}`, error)
}
}
throw new Error(`Invalid code hash: ${codeHash}`)
}