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

fix(cli) - fix wrong condition check in contracts deployment #763

Merged
merged 7 commits into from
Nov 1, 2023
Merged

fix(cli) - fix wrong condition check in contracts deployment #763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
447a959
fix(cli) - fix wrong condition check in contracts deployment
ctrlc03 Oct 22, 2023
ae886a1
Merge branch 'dev' of github.com:privacy-scaling-explorations/maci in…
ctrlc03 Oct 30, 2023
e8064f9
Merge branch 'dev' of github.com:privacy-scaling-explorations/maci in…
ctrlc03 Oct 30, 2023
3633678
fix(cli-create) - add more debug logs to check test failure
ctrlc03 Oct 30, 2023
a58299d
fix(deploy) - add debug on integration tests
ctrlc03 Oct 30, 2023
299bd68
fix(deploy) - fix type errors
ctrlc03 Oct 30, 2023
08da813
fix(deploy-integration tests) - re-add maci address console.log
ctrlc03 Oct 30, 2023
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
34 changes: 18 additions & 16 deletions cli/ts/create.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,23 +64,23 @@ const create = async (args: any) => {
return
}

const TopupCreditContract = await deployTopupCreditContract()
console.log('TopupCredit:', TopupCreditContract.address)

// Initial voice credits
const initialVoiceCredits = args.initial_voice_credits ? args.initial_voice_credits : DEFAULT_INITIAL_VOICE_CREDITS
const topupCreditContract = await deployTopupCreditContract()
console.log('TopupCredit:', topupCreditContract.address)

// Initial voice credit proxy contract
const initialVoiceCreditProxy = args.initial_vc_proxy

// Whether we should deploy a ConstantInitialVoiceCreditProxy
if (initialVoiceCreditProxy != undefined && initialVoiceCredits != undefined) {
if (initialVoiceCreditProxy && args.initial_voice_credits) {
console.error('Error: only one of the following can be specified: the initial voice credit proxy or the amount of initial voice credits.')
return
}

let initialVoiceCreditProxyContractAddress
if (initialVoiceCreditProxy == undefined) {
let initialVoiceCreditProxyContractAddress: string
if (!initialVoiceCreditProxy) {
// check if we have the amount of credits to set, or use the default
const initialVoiceCredits = args.initial_voice_credits ? args.initial_voice_credits : DEFAULT_INITIAL_VOICE_CREDITS
Copy link
Member

Choose a reason for hiding this comment

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

Keep in mind that DEFAULT_INITIAL_VOICE_CREDITS is being imported fromdefaults.ts. Even though it's intended to serve as a default value here, there's no guarantee that it will always be defined in the external file. It's worth ensuring that there's a safeguard or check in place to handle potential undefined values from external imports.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if someone goes out of their way to remove the entry from defaults.ts then the code will not compile as create.ts it's importing something that does not exist

Copy link
Member

Choose a reason for hiding this comment

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

yeah this isn't a major issue, but optionally, for the sake of robustness, you might consider adding a safeguard for DEFAULT_INITIAL_VOICE_CREDITS. Even if defaults.ts is present and being imported, there's no strict guarantee that DEFAULT_INITIAL_VOICE_CREDITS will be consistently defined within that file. A safeguard or a check could make the code more resilient and aligned with best practices of treating external dependencies with caution.


// Deploy a ConstantInitialVoiceCreditProxy contract
const c = await deployConstantInitialVoiceCreditProxy(
initialVoiceCredits,
Expand All @@ -91,23 +91,25 @@ const create = async (args: any) => {
initialVoiceCreditProxyContractAddress = initialVoiceCreditProxy
}

console.log("Initial voice credit proxy", initialVoiceCreditProxyContractAddress)
// Signup gatekeeper contract
const signupGatekeeper = args.signup_gatekeeper

let signUpGatekeeperAddress
if (signupGatekeeper == undefined) {
let signUpGatekeeperAddress: string
if (!signupGatekeeper) {
// Deploy a FreeForAllGatekeeper contract
const c = await deployFreeForAllSignUpGatekeeper(true)
signUpGatekeeperAddress = c.address
} else {
signUpGatekeeperAddress = signupGatekeeper
}


console.log("Signup gatekeeper", signUpGatekeeperAddress)
const verifierContract = await deployVerifier(true)
console.log("Verifier", verifierContract.address)

const vkRegistryContractAddress = args.vk_registry ? args.vk_registry: contractAddrs["VkRegistry"]

console.log("VkRegistry", vkRegistryContractAddress)
const {
maciContract,
stateAqContract,
Expand All @@ -118,18 +120,18 @@ const create = async (args: any) => {
initialVoiceCreditProxyContractAddress,
verifierContract.address,
vkRegistryContractAddress,
TopupCreditContract.address
topupCreditContract.address
)

console.log('MACI:', maciContract.address)

contractAddrs['InitialVoiceCreditProxy'] = initialVoiceCreditProxyContractAddress
contractAddrs['SignUpGatekeeper'] = signUpGatekeeperAddress
contractAddrs['Verifier'] = verifierContract.address
contractAddrs['MACI'] = maciContract.address
contractAddrs['StateAq'] = stateAqContract.address
contractAddrs['PollFactory'] = pollFactoryContract.address
contractAddrs['TopupCredit'] = TopupCreditContract.address
contractAddrs['TopupCredit'] = topupCreditContract.address
contractAddrs['PoseidonT3'] = poseidonAddrs[0]
contractAddrs['PoseidonT4'] = poseidonAddrs[1]
contractAddrs['PoseidonT5'] = poseidonAddrs[2]
Expand Down
15 changes: 3 additions & 12 deletions integrationTests/ts/__tests__/suites.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,23 +41,15 @@ const executeSuite = async (data: any, expect: any) => {
const config = loadYaml()
const coordinatorKeypair = new Keypair()

const maciState = new MaciState(
coordinatorKeypair,
config.constants.maci.stateTreeDepth,
config.constants.maci.messageTreeDepth,
config.constants.maci.voteOptionTreeDepth,
config.constants.maci.maxVoteOptions,
)
const maciState = new MaciState()

const deployVkRegistryCommand = `node build/index.js deployVkRegistry`
const vkDeployOutput = exec(deployVkRegistryCommand)
const vkAddressMatch = vkDeployOutput.stdout.trim().match(/(0x[a-fA-F0-9]{40})/)
if (!vkAddressMatch) {
console.log(vkDeployOutput)
return false
}
const vkAddress = vkAddressMatch[1]
console.log(vkAddress)

let subsidyZkeyFilePath
let subsidyWitnessCalculatorPath
Expand Down Expand Up @@ -122,13 +114,12 @@ const executeSuite = async (data: any, expect: any) => {
treeDepths.voteOptionTreeDepth = config.constants.maci.voteOptionTreeDepth

const maxValues = {} as MaxValues
maxValues.maxUsers = config.constants.maci.maxUsers
maxValues.maxMessages = config.constants.maci.maxMessages
maxValues.maxVoteOptions = config.constants.maci.maxVoteOptions
const messageBatchSize = 5 ** config.constants.poll.messageBatchDepth
maciState.deployPoll(
config.constants.poll.duration,
(Date.now() + (config.constants.poll.duration * 60000)),
BigInt((Date.now() + (config.constants.poll.duration * 60000))),
maxValues,
treeDepths,
messageBatchSize,
Expand Down Expand Up @@ -160,7 +151,7 @@ const executeSuite = async (data: any, expect: any) => {
maciState.signUp(
userKeypair.pubKey,
BigInt(config.constants.maci.initialVoiceCredits),
Date.now()
BigInt(Date.now())
)
}

Expand Down
Loading