From 79c176f4e8dca17133d78f7b86cf1ae4e80f1327 Mon Sep 17 00:00:00 2001 From: bogdan-rosianu <51945539+bogdan-rosianu@users.noreply.github.com> Date: Mon, 19 Feb 2024 16:14:41 +0200 Subject: [PATCH] check accounts and tokens ownership (#5) * check accounts owners as well * fix + logging * test log * additional log * logs * logs changes * logs changes * revert * push build * fixes * fixes * update regex * update re * fix account extraction * more logs + fix * fix * added token ownership checks * fix build after merge * cleanup * single account onwer + added default admin wallet * apiUrl refactoring * some more smaller refactorings * ownerAddress instead of owner * renamings + small refactor * renames and refactorings * return address in case of user wallet * fix tokens regex --------- Co-authored-by: tanghel --- action/index.cjs | 155 ++++++++++++++++++++++++++++++++++------- src/bot.ts | 178 ++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 280 insertions(+), 53 deletions(-) diff --git a/action/index.cjs b/action/index.cjs index 6ea67fb..ae6ea41 100644 --- a/action/index.cjs +++ b/action/index.cjs @@ -187982,6 +187982,7 @@ const robot = (app) => { app.on(['pull_request.opened', 'pull_request.synchronize'], async (context) => { try { const repo = context.repo(); + console.log("Starting processing the assets ownership checks"); async function createComment(body) { try { await context.octokit.issues.createComment({ @@ -187996,17 +187997,17 @@ const robot = (app) => { console.error(error); } } - async function getOwners(files) { + async function getIdentityOwners(files) { const originalOwners = []; const newOwners = []; const networkPath = network === 'mainnet' ? '' : `${network}/`; - const infoJsonUrl = `https://raw.githubusercontent.com/multiversx/mx-assets/master/${networkPath}identities/${identity}/info.json`; + const infoJsonUrl = `https://raw.githubusercontent.com/multiversx/mx-assets/master/${networkPath}identities/${asset}/info.json`; // we try to read the contents of the info.json file const { data: infoFromMaster } = await axios_1.default.get(infoJsonUrl, { validateStatus: status => [200, 404].includes(status) }); if (infoFromMaster && typeof infoFromMaster === 'object' && infoFromMaster['owners']) { originalOwners.push(...infoFromMaster.owners); } - const infoJsonFile = files.find(x => x.filename.endsWith(`/${identity}/info.json`)); + const infoJsonFile = files.find(x => x.filename.endsWith(`/${asset}/info.json`)); if (infoJsonFile) { const { data: infoFromPullRequest } = await axios_1.default.get(infoJsonFile.raw_url); if (infoFromPullRequest && typeof infoFromPullRequest === 'object' && infoFromPullRequest['owners']) { @@ -188025,13 +188026,7 @@ const robot = (app) => { console.log(`Names of changed files: ${printableFilenames}. original owners=${originalOwners}. new owners: ${newOwners}`); const allOwners = []; const allOwnersToCheck = [mainOwner, ...extraOwners]; - let apiUrl = 'https://next-api.multiversx.com'; - if (network === 'devnet') { - apiUrl = 'https://devnet-api.multiversx.com'; - } - else if (network === 'testnet') { - apiUrl = 'https://testnet-api.multiversx.com'; - } + const apiUrl = getApiUrl(); for (const owner of allOwnersToCheck) { if (new out_1.Address(owner).isContractAddress()) { const ownerResult = await axios_1.default.get(`${apiUrl}/accounts/${owner}?extract=ownerAddress`); @@ -188043,18 +188038,65 @@ const robot = (app) => { } return [...new Set(allOwners)]; } + async function getAccountOwner(account) { + const accountOwner = account; + if (new out_1.Address(accountOwner).isContractAddress()) { + return getAccountOwnerFromApi(accountOwner); + } + return accountOwner; + } + async function getAccountOwnerFromApi(address) { + const apiUrl = getApiUrl(); + const accountOwnerResponse = await axios_1.default.get(`${apiUrl}/accounts/${address}?extract=ownerAddress`); + if (accountOwnerResponse && accountOwnerResponse.data) { + return accountOwnerResponse.data; + } + return ''; + } + async function getTokenOwner(token) { + // since the token owner can be changed at protocol level at any time, it's enough to check the ownership of the token, + // without checking any previous owners + const apiUrl = getApiUrl(); + const tokenOwner = await getTokenOwnerFromApi(token, apiUrl); + if (new out_1.Address(tokenOwner).isContractAddress()) { + const ownerResult = await axios_1.default.get(`${apiUrl}/tokens/${token}?extract=ownerAddress`); + return ownerResult.data; + } + return tokenOwner; + } + async function getTokenOwnerFromApi(token, apiUrl) { + const tokenOwnerResponse = await axios_1.default.get(`${apiUrl}/tokens/${token}?extract=owner`); + if (tokenOwnerResponse && tokenOwnerResponse.data) { + return tokenOwnerResponse.data; + } + return ''; + } + function getApiUrl() { + switch (network) { + case 'mainnet': + return 'https://next-api.multiversx.com'; + case 'devnet': + return 'https://devnet-api.multiversx.com'; + case 'testnet': + return 'https://testnet-api.multiversx.com'; + } + throw new Error(`Invalid network: ${network}`); + } function getDistinctNetworks(fileNames) { const networks = fileNames.map(fileName => getNetwork(fileName)).filter(x => x !== undefined); return [...new Set(networks)]; } function getNetwork(fileName) { - if (fileName.startsWith('identities')) { + const mainnetRegex = /^(identities|accounts|tokens)\b/; + const testnetRegex = /^testnet\/(identities|accounts|tokens)\b/; + const devnetRegex = /^devnet\/(identities|accounts|tokens)\b/; + if (mainnetRegex.test(fileName)) { return 'mainnet'; } - if (fileName.startsWith('testnet/identities')) { + if (testnetRegex.test(fileName)) { return 'testnet'; } - if (fileName.startsWith('devnet/identities')) { + if (devnetRegex.test(fileName)) { return 'devnet'; } return undefined; @@ -188066,6 +188108,20 @@ const robot = (app) => { .filter(x => x); return [...new Set(identities)]; } + function getDistinctAccounts(fileNames) { + const regex = /accounts\/(.*?).json/; + const accounts = fileNames + .map(x => regex.exec(x)?.at(1)) + .filter(x => x); + return [...new Set(accounts)]; + } + function getDistinctTokens(fileNames) { + const regex = /tokens\/(.*?)\/info.json/; + const tokens = fileNames + .map(x => regex.exec(x)?.at(1)) + .filter(x => x); + return [...new Set(tokens)]; + } async function fail(reason) { await createComment(reason); console.error(reason); @@ -188075,7 +188131,7 @@ const robot = (app) => { const signature = /[0-9a-fA-F]{128}/.exec(body)?.at(0); if (signature) { const verifyResult = await verifySignature(signature, address, message); - console.log(`verifying signature for address ${address}, message ${message}, and signature ${signature}. Result=${verifyResult}`); + console.log(`Verifying signature for address ${address}, message ${message}, and signature ${signature}. Result=${verifyResult}`); return verifyResult; } const txHash = /[0-9a-fA-F]{64}/.exec(body)?.at(0); @@ -188120,6 +188176,7 @@ const robot = (app) => { const { data: pullRequest } = await axios_1.default.get(`https://api.github.com/repos/multiversx/mx-assets/pulls/${context.pullRequest().pull_number}`); const state = pullRequest.state; if (state === 'closed' || state === 'locked' || state === 'draft') { + await fail(`Invalid PR state: ${state}`); return 'invalid event payload'; } const data = await context.octokit.repos.compareCommits({ @@ -188134,12 +188191,36 @@ const robot = (app) => { if (!changedFiles?.length) { return 'no change'; } - const distinctIdentities = getDistinctIdentities(changedFiles.map(x => x.filename)); - if (distinctIdentities.length === 0) { + let checkMode = 'identity'; + const changedFilesNames = changedFiles.map(x => x.filename); + const distinctStakingIdentities = getDistinctIdentities(changedFilesNames); + const distinctAccounts = getDistinctAccounts(changedFilesNames); + const distinctTokens = getDistinctTokens(changedFilesNames); + const countDistinctStakingIdentities = distinctStakingIdentities.length; + if (countDistinctStakingIdentities) { + checkMode = 'identity'; + } + const countDistinctAccounts = distinctAccounts.length; + if (countDistinctAccounts) { + checkMode = 'account'; + } + const countDistinctTokens = distinctTokens.length; + if (countDistinctTokens) { + checkMode = 'token'; + } + const sumOfAllChangedAssets = countDistinctAccounts + countDistinctStakingIdentities + countDistinctTokens; + if (sumOfAllChangedAssets === 0) { + await fail("No identity, token or account changed."); + return; + } + if (sumOfAllChangedAssets > 1) { + await fail("Only one identity, token or account update at a time."); return; } + const distinctIdentities = [...distinctStakingIdentities, ...distinctAccounts, ...distinctTokens]; const distinctNetworks = getDistinctNetworks(changedFiles.map(x => x.filename)); if (distinctNetworks.length === 0) { + await fail("No network changed."); return; } const comments = await context.octokit.issues.listComments({ @@ -188150,13 +188231,14 @@ const robot = (app) => { }); const body = pullRequest.body || ''; const bodies = [...comments.data.map(x => x.body || ''), body]; - const adminAddress = process.env.ADMIN_ADDRESS; - if (adminAddress) { - const invalidAddresses = await multiVerify(bodies, [adminAddress], commitShas); - if (invalidAddresses && invalidAddresses.length === 0) { - await createComment(`Signature OK. Verified that the latest commit hash \`${lastCommitSha}\` was signed using the admin wallet address`); - return; - } + let adminAddress = process.env.ADMIN_ADDRESS; + if (!adminAddress) { + adminAddress = 'erd1cevsw7mq5uvqymjqzwqvpqtdrhckehwfz99n7praty3y7q2j7yps842mqh'; + } + const invalidAddressesForAdminChecks = await multiVerify(bodies, [adminAddress], commitShas); + if (invalidAddressesForAdminChecks && invalidAddressesForAdminChecks.length === 0) { + await createComment(`Signature OK. Verified that the latest commit hash \`${lastCommitSha}\` was signed using the admin wallet address`); + return; } if (distinctIdentities.length > 1) { await fail('Only one identity must be edited at a time'); @@ -188166,14 +188248,33 @@ const robot = (app) => { await fail('Only one network must be edited at a time'); return; } - const identity = distinctIdentities[0]; + const asset = distinctIdentities[0]; + if (!asset) { + await fail('No asset update detected'); + return; + } const network = distinctNetworks[0]; - let owners = await getOwners(changedFiles); + let owners; + switch (checkMode) { + case 'identity': + owners = await getIdentityOwners(changedFiles); + break; + case 'account': + const accountOwner = await getAccountOwner(asset); + owners = [accountOwner]; + break; + case 'token': + const tokenOwner = await getTokenOwner(asset); + owners = [tokenOwner]; + break; + default: + owners = []; + } if (owners.length === 0) { await fail('No owners identified'); return; } - console.log(`Addresses to check ownership for: ${owners}`); + console.log(`Asset owners. check mode=${checkMode}. value=${owners}`); const invalidAddresses = await multiVerify(bodies, owners, commitShas); if (!invalidAddresses) { await fail('Failed to verify owners'); @@ -188186,7 +188287,7 @@ const robot = (app) => { return; } else { - const ownersDescription = owners.map(address => `\`${address}\``).join('\n'); + const ownersDescription = owners.map((address) => `\`${address}\``).join('\n'); await createComment(`Signature OK. Verified that the latest commit hash \`${lastCommitSha}\` was signed using the wallet ${addressDescription}: \n${ownersDescription}`); } console.info('successfully reviewed', pullRequest.html_url); diff --git a/src/bot.ts b/src/bot.ts index 20affc2..9d2cbb2 100644 --- a/src/bot.ts +++ b/src/bot.ts @@ -9,6 +9,7 @@ export const robot = (app: Probot) => { async (context) => { try { const repo = context.repo(); + console.log("Starting processing the assets ownership checks"); async function createComment(body: string) { try { @@ -24,11 +25,12 @@ export const robot = (app: Probot) => { } } - async function getOwners(files: { filename: string, raw_url: string }[]): Promise { + async function getIdentityOwners(files: { filename: string, raw_url: string }[]): Promise { const originalOwners: string[] = []; const newOwners: string[] = []; const networkPath = network === 'mainnet' ? '' : `${network}/`; - const infoJsonUrl = `https://raw.githubusercontent.com/multiversx/mx-assets/master/${networkPath}identities/${identity}/info.json`; + + const infoJsonUrl = `https://raw.githubusercontent.com/multiversx/mx-assets/master/${networkPath}identities/${asset}/info.json`; // we try to read the contents of the info.json file const { data: infoFromMaster } = await axios.get(infoJsonUrl, { validateStatus: status => [200, 404].includes(status) }); @@ -37,7 +39,7 @@ export const robot = (app: Probot) => { originalOwners.push(...infoFromMaster.owners); } - const infoJsonFile = files.find(x => x.filename.endsWith(`/${identity}/info.json`)); + const infoJsonFile = files.find(x => x.filename.endsWith(`/${asset}/info.json`)); if (infoJsonFile) { const { data: infoFromPullRequest } = await axios.get(infoJsonFile.raw_url); @@ -61,12 +63,7 @@ export const robot = (app: Probot) => { const allOwners: string[] = []; const allOwnersToCheck = [mainOwner, ...extraOwners]; - let apiUrl = 'https://next-api.multiversx.com'; - if (network === 'devnet') { - apiUrl = 'https://devnet-api.multiversx.com'; - } else if (network === 'testnet') { - apiUrl = 'https://testnet-api.multiversx.com'; - } + const apiUrl = getApiUrl(); for (const owner of allOwnersToCheck) { if (new Address(owner).isContractAddress()) { @@ -80,6 +77,61 @@ export const robot = (app: Probot) => { return [...new Set(allOwners)]; } + async function getAccountOwner(account: string): Promise { + const accountOwner = account; + if (new Address(accountOwner).isContractAddress()) { + return getAccountOwnerFromApi(accountOwner); + } + + return accountOwner; + } + + async function getAccountOwnerFromApi(address: string): Promise { + const apiUrl = getApiUrl(); + const accountOwnerResponse = await axios.get(`${apiUrl}/accounts/${address}?extract=ownerAddress`); + if (accountOwnerResponse && accountOwnerResponse.data) { + return accountOwnerResponse.data; + } + + return ''; + } + + async function getTokenOwner(token: string): Promise { + // since the token owner can be changed at protocol level at any time, it's enough to check the ownership of the token, + // without checking any previous owners + const apiUrl = getApiUrl(); + + const tokenOwner = await getTokenOwnerFromApi(token, apiUrl); + if (new Address(tokenOwner).isContractAddress()) { + const ownerResult = await axios.get(`${apiUrl}/tokens/${token}?extract=ownerAddress`); + return ownerResult.data; + } + + return tokenOwner; + } + + async function getTokenOwnerFromApi(token: string, apiUrl: string): Promise { + const tokenOwnerResponse = await axios.get(`${apiUrl}/tokens/${token}?extract=owner`); + if (tokenOwnerResponse && tokenOwnerResponse.data) { + return tokenOwnerResponse.data; + } + + return ''; + } + + function getApiUrl() { + switch (network) { + case 'mainnet': + return 'https://next-api.multiversx.com'; + case 'devnet': + return 'https://devnet-api.multiversx.com'; + case 'testnet': + return 'https://testnet-api.multiversx.com'; + } + + throw new Error(`Invalid network: ${network}`); + } + function getDistinctNetworks(fileNames: string[]) { const networks = fileNames.map(fileName => getNetwork(fileName)).filter(x => x !== undefined); @@ -87,21 +139,26 @@ export const robot = (app: Probot) => { } function getNetwork(fileName: string): 'mainnet' | 'devnet' | 'testnet' | undefined { - if (fileName.startsWith('identities')) { + const mainnetRegex = /^(identities|accounts|tokens)\b/; + const testnetRegex = /^testnet\/(identities|accounts|tokens)\b/; + const devnetRegex = /^devnet\/(identities|accounts|tokens)\b/; + + if (mainnetRegex.test(fileName)) { return 'mainnet'; } - if (fileName.startsWith('testnet/identities')) { + if (testnetRegex.test(fileName)) { return 'testnet'; } - if (fileName.startsWith('devnet/identities')) { + if (devnetRegex.test(fileName)) { return 'devnet'; } return undefined; } + function getDistinctIdentities(fileNames: string[]) { const regex = /identities\/(.*?)\//; @@ -112,6 +169,26 @@ export const robot = (app: Probot) => { return [...new Set(identities)]; } + function getDistinctAccounts(fileNames: string[]) { + const regex = /accounts\/(.*?).json/; + + const accounts = fileNames + .map(x => regex.exec(x)?.at(1)) + .filter(x => x); + + return [...new Set(accounts)]; + } + + function getDistinctTokens(fileNames: string[]) { + const regex = /tokens\/(.*?)\/info.json/; + + const tokens = fileNames + .map(x => regex.exec(x)?.at(1)) + .filter(x => x); + + return [...new Set(tokens)]; + } + async function fail(reason: string) { await createComment(reason); console.error(reason); @@ -122,7 +199,7 @@ export const robot = (app: Probot) => { const signature = /[0-9a-fA-F]{128}/.exec(body)?.at(0); if (signature) { const verifyResult = await verifySignature(signature, address, message); - console.log(`verifying signature for address ${address}, message ${message}, and signature ${signature}. Result=${verifyResult}`); + console.log(`Verifying signature for address ${address}, message ${message}, and signature ${signature}. Result=${verifyResult}`); return verifyResult; } @@ -181,6 +258,7 @@ export const robot = (app: Probot) => { const state = pullRequest.state; if (state === 'closed' || state === 'locked' || state === 'draft') { + await fail(`Invalid PR state: ${state}`); return 'invalid event payload'; } @@ -200,13 +278,40 @@ export const robot = (app: Probot) => { return 'no change'; } - const distinctIdentities = getDistinctIdentities(changedFiles.map(x => x.filename)); - if (distinctIdentities.length === 0) { + let checkMode = 'identity'; + const changedFilesNames = changedFiles.map(x => x.filename); + const distinctStakingIdentities = getDistinctIdentities(changedFilesNames); + const distinctAccounts = getDistinctAccounts(changedFilesNames); + const distinctTokens = getDistinctTokens(changedFilesNames); + + const countDistinctStakingIdentities = distinctStakingIdentities.length; + if (countDistinctStakingIdentities) { + checkMode = 'identity'; + } + const countDistinctAccounts = distinctAccounts.length; + if (countDistinctAccounts) { + checkMode = 'account'; + } + const countDistinctTokens = distinctTokens.length; + if (countDistinctTokens) { + checkMode = 'token'; + } + + const sumOfAllChangedAssets = countDistinctAccounts + countDistinctStakingIdentities + countDistinctTokens; + if (sumOfAllChangedAssets === 0) { + await fail("No identity, token or account changed."); return; } + if (sumOfAllChangedAssets > 1) { + await fail("Only one identity, token or account update at a time."); + return; + } + + const distinctIdentities = [...distinctStakingIdentities, ...distinctAccounts, ...distinctTokens]; const distinctNetworks = getDistinctNetworks(changedFiles.map(x => x.filename)); if (distinctNetworks.length === 0) { + await fail("No network changed."); return; } @@ -221,14 +326,15 @@ export const robot = (app: Probot) => { const bodies = [...comments.data.map(x => x.body || ''), body]; - const adminAddress = process.env.ADMIN_ADDRESS; + let adminAddress = process.env.ADMIN_ADDRESS; + if (!adminAddress) { + adminAddress = 'erd1cevsw7mq5uvqymjqzwqvpqtdrhckehwfz99n7praty3y7q2j7yps842mqh'; + } - if (adminAddress) { - const invalidAddresses = await multiVerify(bodies, [adminAddress], commitShas); - if (invalidAddresses && invalidAddresses.length === 0) { - await createComment(`Signature OK. Verified that the latest commit hash \`${lastCommitSha}\` was signed using the admin wallet address`); - return; - } + const invalidAddressesForAdminChecks = await multiVerify(bodies, [adminAddress], commitShas); + if (invalidAddressesForAdminChecks && invalidAddressesForAdminChecks.length === 0) { + await createComment(`Signature OK. Verified that the latest commit hash \`${lastCommitSha}\` was signed using the admin wallet address`); + return; } if (distinctIdentities.length > 1) { @@ -241,16 +347,36 @@ export const robot = (app: Probot) => { return; } - const identity = distinctIdentities[0]; + const asset = distinctIdentities[0]; + if (!asset) { + await fail('No asset update detected'); + return; + } const network = distinctNetworks[0]; - let owners = await getOwners(changedFiles); + let owners: string[]; + switch (checkMode) { + case 'identity': + owners = await getIdentityOwners(changedFiles); + break; + case 'account': + const accountOwner = await getAccountOwner(asset); + owners = [accountOwner]; + break; + case 'token': + const tokenOwner = await getTokenOwner(asset); + owners = [tokenOwner]; + break; + default: + owners = []; + } + if (owners.length === 0) { await fail('No owners identified'); return; } - console.log(`Addresses to check ownership for: ${owners}`); + console.log(`Asset owners. check mode=${checkMode}. value=${owners}`); const invalidAddresses = await multiVerify(bodies, owners, commitShas); if (!invalidAddresses) { await fail('Failed to verify owners'); @@ -264,7 +390,7 @@ export const robot = (app: Probot) => { await fail(`Please provide a signature for the latest commit sha: \`${lastCommitSha}\` which must be signed with the owner wallet ${addressDescription}: \n${invalidAddressesDescription}`); return; } else { - const ownersDescription = owners.map(address => `\`${address}\``).join('\n'); + const ownersDescription = owners.map((address: any) => `\`${address}\``).join('\n'); await createComment(`Signature OK. Verified that the latest commit hash \`${lastCommitSha}\` was signed using the wallet ${addressDescription}: \n${ownersDescription}`); }