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

refactor(core-magistrate): allow multiple ports in bridgechain schema #3504

Merged
merged 7 commits into from
Feb 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 5 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,11 @@ jobs:
with:
node-version: ${{ matrix.node-version }}
- name: Install and build packages
run: yarn setup && cd __tests__/e2e && ../../node_modules/.bin/tsc && yarn cache clean && yarn install --frozen-lockfile
run: yarn setup && cd __tests__/e2e && yarn cache clean && yarn install --frozen-lockfile
- name: Build test helpers
run: cd __tests__/helpers && echo "{}" > tsconfig.json && ../../node_modules/.bin/tsc || true
- name: Build test utils
run: cd __tests__/utils && echo "{}" > tsconfig.json && ../../node_modules/.bin/tsc || true
- name: Docker compose up
run: cd __tests__/e2e/lib/config && docker-compose up -d
- name: Wait 20 sec for docker containers to be up and nodes be running
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ const bridgechainRegistrationAsset = {
genesisHash: "127e6fbfe24a750e72930c220a8e138275656b8e5d8f48a98c3c92df2caba935",
bridgechainRepository: "http://www.repository.com/myorg/myrepo",
bridgechainAssetRepository: "http://www.repository.com/myorg/myassetrepo",
ports: { "@arkecosystem/core-api": 12345 },
ports: { "@arkecosystem/core-api": 12345, "custom/api": 3456 },
};

const bridgechainUpdateAsset = {
bridgechainId: bridgechainRegistrationAsset.genesisHash,
seedNodes: [
"75.125.224.71",
],
ports: { "@arkecosystem/core-api": 54321 },
ports: { "@arkecosystem/core-api": 54321, "custom/other-api": 9876 },
bridgechainRepository: "http://www.newrepository.com/neworg/newrepo",
bridgechainAssetRepository: "http://www.newrepository.com/neworg/newassetrepo",
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Handlers } from "@arkecosystem/core-transactions";
import { Managers, Utils } from "@arkecosystem/crypto";
import {
BridgechainAlreadyRegisteredError,
PortKeyMustBeValidPackageNameError,
WalletIsNotBusinessError,
} from "../../../../packages/core-magistrate-transactions/src/errors";
import {
Expand Down Expand Up @@ -121,6 +122,30 @@ describe("should test marketplace transaction handlers", () => {
bridgechainRegistrationHandler.throwIfCannotBeApplied(actual.build(), senderWallet, walletManager),
).rejects.toThrowError(BridgechainAlreadyRegisteredError);
});

it.each([["@invalid/UPPERCASE"], ["@invalid/char)"]])(
"should throw because ports contains invalid package name",
async invalidName => {
const actual = bridgechainRegistrationBuilder
.bridgechainRegistrationAsset({
...bridgechainRegistrationAsset1,
ports: {
...bridgechainRegistrationAsset1.ports,
[invalidName]: 4444,
},
})
.nonce("2")
.sign("clay harbor enemy utility margin pretty hub comic piece aerobic umbrella acquire");

await expect(
bridgechainRegistrationHandler.throwIfCannotBeApplied(
actual.build(),
senderWallet,
walletManager,
),
).rejects.toThrowError(PortKeyMustBeValidPackageNameError);
},
);
});

describe("applyToSender tests", () => {
Expand Down
21 changes: 21 additions & 0 deletions __tests__/unit/core-magistrate/handlers/bridgechain-update.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Managers, Utils } from "@arkecosystem/crypto";
import {
BridgechainIsNotRegisteredByWalletError,
BusinessIsNotRegisteredError,
PortKeyMustBeValidPackageNameError,
} from "../../../../packages/core-magistrate-transactions/src/errors";
import {
BridgechainRegistrationTransactionHandler,
Expand Down Expand Up @@ -128,6 +129,26 @@ describe("Bridgechain update handler", () => {
await bridgechainRegistrationHandler.applyToSender(bridgechainRegistration.build(), walletManager);
});

it.each([["@invalid/UPPERCASE"], ["@invalid/char)"]])(
"should throw because ports contains invalid package name",
async invalidName => {
const actual = bridgechainUpdateBuilder
.bridgechainUpdateAsset({
...bridgechainUpdateAsset1,
ports: {
...bridgechainUpdateAsset1.ports,
[invalidName]: 4444,
},
})
.nonce("3")
.sign("clay harbor enemy utility margin pretty hub comic piece aerobic umbrella acquire");

await expect(
bridgechainUpdateHandler.throwIfCannotBeApplied(actual.build(), senderWallet, walletManager),
).rejects.toThrowError(PortKeyMustBeValidPackageNameError);
},
);

it("should not throw because bridgechain is registered", async () => {
const actual = bridgechainUpdateBuilder
.bridgechainUpdateAsset(bridgechainUpdateAsset1)
Expand Down
5 changes: 4 additions & 1 deletion __tests__/unit/core-magistrate/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ export const bridgechainRegistrationAsset1: IBridgechainRegistrationAsset = {
genesisHash: "127e6fbfe24a750e72930c220a8e138275656b8e5d8f48a98c3c92df2caba935",
bridgechainRepository: "http://www.repository.com/myorg/myrepo",
bridgechainAssetRepository: "http://www.repository.com/myorg/myassetrepo",
ports: { "@arkecosystem/core-api": 12345 },
ports: {
"@arkecosystem/core-api": 12345,
"@custom/api": 3333,
},
};

export const bridgechainRegistrationAsset2: IBridgechainRegistrationAsset = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,5 +250,50 @@ describe("Bridgechain registration transaction", () => {
expect(error).not.toBeUndefined();
});
});

describe("should test edge cases for ports", () => {
it.each([["empty is invalid", {}], ["needs to include core-api", { "@arkecosystem/not-core-api": 123 }]])(
"should fail to validate for '%s' case",
(_, ports) => {
const bridgechainRegistration = builder
.bridgechainRegistrationAsset({
name: "google",
genesisHash: "000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f",
bridgechainRepository: "http://www.repository.com/google/syzkaller",
bridgechainAssetRepository: "http://www.repository.com/google/asset",
seedNodes: ["66.102.0.0"],
ports,
})
.sign("passphrase");

const struct = bridgechainRegistration.getStruct();
const { error } = Validation.validator.validate(transactionSchema, struct);
console.log(JSON.stringify(struct));
expect(error).not.toBeUndefined();
},
);

it.each([
[{ "@arkecosystem/core-api": 123 }],
[{ "@arkecosystem/core-api": 123, "@valid/pa_ckage-name": 124 }],
[{ "@arkecosystem/core-api": 123, "valid-package_name": 124 }],
[{ "@arkecosystem/core-api": 123, "@invalid/UPPERCASE": 124 }],
[{ "@arkecosystem/core-api": 123, "@invalid/char)": 124 }],
])("should validate", ports => {
const bridgechainRegistration = builder
.bridgechainRegistrationAsset({
name: "google",
genesisHash: "000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f",
bridgechainRepository: "http://www.repository.com/google/syzkaller",
bridgechainAssetRepository: "http://www.repository.com/google/asset",
seedNodes: ["66.102.0.0"],
ports,
})
.sign("passphrase");

const { error } = Validation.validator.validate(transactionSchema, bridgechainRegistration.getStruct());
expect(error).toBeUndefined();
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ export const seedNodesSchema = {

export const portsSchema = {
type: "object",
maxProperties: 1,
maxProperties: 10,
minProperties: 1,
required: ["@arkecosystem/core-api"],
additionalProperties: false,
properties: {
"@arkecosystem/core-api": {
patternProperties: {
// just allow anything within length limitation of npm package name, more
// precise validation will be done in transaction handler
"^.{1,214}$": {
type: "integer",
minimum: 0,
maximum: 65535,
Expand Down
6 changes: 6 additions & 0 deletions packages/core-magistrate-transactions/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,9 @@ export class GenesisHashAlreadyRegisteredError extends Errors.TransactionError {
super("Failed to apply transaction, because genesis hash is already registered by another bridgechain.");
}
}

export class PortKeyMustBeValidPackageNameError extends Errors.TransactionError {
constructor() {
super("Failed to apply transaction, because the package name(s) defined in ports is not valid.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ import {
BridgechainAlreadyRegisteredError,
BusinessIsResignedError,
GenesisHashAlreadyRegisteredError,
PortKeyMustBeValidPackageNameError,
WalletIsNotBusinessError,
} from "../errors";
import { MagistrateApplicationEvents } from "../events";
import { IBridgechainWalletAttributes, IBusinessWalletAttributes } from "../interfaces";
import { BusinessRegistrationTransactionHandler } from "./business-registration";
import { MagistrateTransactionHandler } from "./magistrate-handler";
import { packageNameRegex } from "./utils";

export class BridgechainRegistrationTransactionHandler extends MagistrateTransactionHandler {
public getConstructor(): Transactions.TransactionConstructor {
Expand Down Expand Up @@ -100,6 +102,12 @@ export class BridgechainRegistrationTransactionHandler extends MagistrateTransac
}
}

for (const portKey of Object.keys(data.asset.bridgechainRegistration.ports)) {
if (!packageNameRegex.test(portKey)) {
throw new PortKeyMustBeValidPackageNameError();
}
}

return super.throwIfCannotBeApplied(transaction, wallet, walletManager);
}

Expand All @@ -111,7 +119,7 @@ export class BridgechainRegistrationTransactionHandler extends MagistrateTransac
data: Interfaces.ITransactionData,
pool: TransactionPool.IConnection,
processor: TransactionPool.IProcessor,
): Promise<{ type: string, message: string } | null> {
): Promise<{ type: string; message: string } | null> {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ import {
BridgechainIsResignedError,
BusinessIsNotRegisteredError,
BusinessIsResignedError,
PortKeyMustBeValidPackageNameError,
} from "../errors";
import { MagistrateApplicationEvents } from "../events";
import { IBridgechainWalletAttributes, IBusinessWalletAttributes } from "../interfaces";
import { BridgechainRegistrationTransactionHandler } from "./bridgechain-registration";
import { MagistrateTransactionHandler } from "./magistrate-handler";
import { packageNameRegex } from "./utils";

export class BridgechainUpdateTransactionHandler extends MagistrateTransactionHandler {
public getConstructor(): Transactions.TransactionConstructor {
Expand Down Expand Up @@ -89,6 +91,12 @@ export class BridgechainUpdateTransactionHandler extends MagistrateTransactionHa
throw new BridgechainIsResignedError();
}

for (const portKey of Object.keys(bridgechainUpdate.ports || {})) {
if (!packageNameRegex.test(portKey)) {
throw new PortKeyMustBeValidPackageNameError();
}
}

return super.throwIfCannotBeApplied(transaction, wallet, walletManager);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const packageNameRegex = new RegExp("^[a-z0-9@/.~_-]{1,214}$");