Skip to content

Commit

Permalink
fix(core-p2p): node version should pass min version defined in config…
Browse files Browse the repository at this point in the history
…uration and milestones (#4695)
  • Loading branch information
sebastijankuzner committed Aug 16, 2022
1 parent 64fcbb3 commit bd37119
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 47 deletions.
125 changes: 88 additions & 37 deletions __tests__/unit/core-p2p/utils/is-valid-version.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,53 +5,104 @@ import { Peer } from "@arkecosystem/core-p2p/src/peer";
import { Managers } from "@arkecosystem/crypto";

let peerMock: Peer;
const getOptional = jest.fn().mockReturnValue([]);
const app = {
getTagged: jest.fn().mockReturnValue({
getOptional: jest.fn().mockReturnValue(["^2.6.0"]),
getOptional,
}),
get: jest.fn().mockReturnValue("mainnet"),
} as any;
beforeEach(async () => {
peerMock = new Peer("1.0.0.99", 4002);
});

describe.each([[true], [false]])("isValidVersion", (withMilestones) => {
let spyGetMilestone = jest.spyOn(Managers.configManager, "getMilestone");
beforeEach(() => {
if (withMilestones) {
spyGetMilestone = jest.spyOn(Managers.configManager, "getMilestone").mockReturnValue({
p2p: {
minimumVersions: ["^2.6.0"]
}
});
}
})
afterEach(() => {
spyGetMilestone.mockRestore();
})

it.each([["2.6.0"], ["2.6.666"], ["2.7.0"], ["2.8.0"], ["2.9.0"], ["2.9.934"]])(
"should be a valid version",
(version) => {
peerMock.version = version;
expect(isValidVersion(app, peerMock)).toBeTrue();
},
);
describe("isValidVersion", () => {
it("should return false if version is not defined", () => {
expect(isValidVersion(app, peerMock)).toBeFalse();
});

it.each([
[undefined]
["2.4.0"],
["2.5.0"],
["1.0.0"],
["---aaa"],
["2490"],
[2],
[-10.2],
[{}],
[true],
[() => 1],
["2.0.0.0"],
])("should be an invalid version", (version: any) => {
it.each(["a", "3", "3.6", "-3.4.0", "3.0.0.next.1"])("should return false if version is not valid", (version) => {
peerMock.version = version;
expect(isValidVersion(app, peerMock)).toBeFalse();
});

it.each(["3.0.0", "3.0.0-next.1"])("should return true if minVersion is not set", (version) => {
peerMock.version = version;
expect(isValidVersion(app, peerMock)).toBeTrue();
});

const tests = [
{ network: "mainnet", version: "3.0.0", minVersion: ["3.0.0"], result: true },
{ network: "mainnet", version: "3.0.0", minVersion: ["3.0.0", "2.0.0"], result: true },
{ network: "mainnet", version: "4.0.0", minVersion: [">=3.0.0 || <=5.0.0"], result: true },
{ network: "mainnet", version: "3.0.1", minVersion: ["3.0.0"], result: false },
{ network: "mainnet", version: "3.0.13", minVersion: [">=3.0.0"], result: true },
{ network: "mainnet", version: "4.0.0", minVersion: [">=3.0.0"], result: true },
{ network: "mainnet", version: "3.3.3", minVersion: ["^3.0.0"], result: true },
{ network: "mainnet", version: "4.0.0", minVersion: ["^3.0.0"], result: false },
{ network: "mainnet", version: "4.0.0-next.1", minVersion: [">=3.0.0"], result: false },
{ network: "devnet", version: "4.0.0-next.1", minVersion: [">=3.0.0"], result: true },
{ network: "mainnet", version: "3.0.0", minVersion: [], result: true },
];
it.each(tests)("minVersion in milestones", (test) => {
const spyGet = jest.spyOn(Managers.configManager, "get").mockReturnValue(test.network);
const spyGetMilestone = jest.spyOn(Managers.configManager, "getMilestone").mockReturnValue({
p2p: {
minimumVersions: test.minVersion,
},
});

peerMock.version = test.version;
expect(isValidVersion(app, peerMock)).toEqual(test.result);
expect(spyGet).toBeCalled();
expect(spyGetMilestone).toBeCalled();
});

it.each(tests)("minVersion in config", (test) => {
const spyGet = jest.spyOn(Managers.configManager, "get").mockReturnValue(test.network);
getOptional.mockReturnValue(test.minVersion);

peerMock.version = test.version;
expect(isValidVersion(app, peerMock)).toEqual(test.result);
expect(spyGet).toBeCalled();
expect(getOptional).toBeCalled();
});

it.each([
{
network: "mainnet",
version: "3.0.0",
minConfigVersion: ["3.0.0"],
minMilestonesVersion: ["3.0.0"],
result: true,
},
{
network: "mainnet",
version: "3.0.0",
minConfigVersion: ["4.0.0"],
minMilestonesVersion: ["3.0.0"],
result: false,
},
{
network: "mainnet",
version: "3.0.0",
minConfigVersion: ["3.0.0"],
minMilestonesVersion: ["4.0.0"],
result: false,
},
])("should pass configuration and milestone minVersion", (test) => {
const spyGet = jest.spyOn(Managers.configManager, "get").mockReturnValue(test.network);
const spyGetMilestone = jest.spyOn(Managers.configManager, "getMilestone").mockReturnValue({
p2p: {
minimumVersions: test.minMilestonesVersion,
},
});
getOptional.mockReturnValue(test.minConfigVersion);

peerMock.version = test.version;
expect(isValidVersion(app, peerMock)).toEqual(test.result);
expect(spyGet).toBeCalled();
expect(spyGetMilestone).toBeCalled();
expect(getOptional).toBeCalled();
});
});
38 changes: 33 additions & 5 deletions packages/core-p2p/src/utils/is-valid-version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { Container, Contracts, Providers } from "@arkecosystem/core-kernel";
import { Managers } from "@arkecosystem/crypto";
import semver from "semver";

// todo: review the implementation
export const isValidVersion = (app: Contracts.Kernel.Application, peer: Contracts.P2P.Peer): boolean => {
if (!peer.version) {
return false;
Expand All @@ -12,15 +11,44 @@ export const isValidVersion = (app: Contracts.Kernel.Application, peer: Contract
return false;
}

const includePrerelease: boolean = Managers.configManager.get("network.name") !== "mainnet";

return (
isValidConfigVersion(app, peer.version, includePrerelease) &&
isValidMilestoneVersion(peer.version, includePrerelease)
);
};

const isValidConfigVersion = (
app: Contracts.Kernel.Application,
version: string,
includePrerelease: boolean,
): boolean => {
const configuration = app.getTagged<Providers.PluginConfiguration>(
Container.Identifiers.PluginConfiguration,
"plugin",
"@arkecosystem/core-p2p",
);

const minimumVersions = configuration.getOptional<string[]>("minimumVersions", []);

const includePrerelease: boolean = Managers.configManager.get("network.name") !== "mainnet";
return minimumVersions.some((minimumVersion: string) =>
semver.satisfies(peer.version!, minimumVersion, { includePrerelease }),
);
if (minimumVersions.length > 0) {
return minimumVersions.some((minimumVersion: string) =>
semver.satisfies(version, minimumVersion, { includePrerelease }),
);
}

return true;
};

const isValidMilestoneVersion = (version: string, includePrerelease: boolean): boolean => {
const { p2p } = Managers.configManager.getMilestone();

if (p2p && Array.isArray(p2p.minimumVersions) && p2p.minimumVersions.length > 0) {
return p2p.minimumVersions.some((minimumVersion: string) =>
semver.satisfies(version, minimumVersion, { includePrerelease }),
);
}

return true;
};
2 changes: 1 addition & 1 deletion packages/crypto/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
},
"dependencies": {
"@arkecosystem/crypto-identities": "1.2.0",
"@arkecosystem/crypto-networks": "1.2.0",
"@arkecosystem/crypto-networks": "1.2.1",
"@arkecosystem/utils": "1.3.1",
"ajv": "6.12.6",
"ajv-keywords": "3.4.1",
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@
fast-memoize "^2.5.1"
wif "^2.0.6"

"@arkecosystem/[email protected].0":
version "1.2.0"
resolved "https://registry.npmjs.org/@arkecosystem/crypto-networks/-/crypto-networks-1.2.0.tgz#7f00d8f2421923ab779b14c3d60b092c5484b44f"
integrity sha512-ZytgxSgP4lfOUdQLvY6PyWkhvx6//V24MKv/faom9preIiamEfwEHLobFRs0cr5F1wSD7brMrLN/KcCkUw2Itw==
"@arkecosystem/[email protected].1":
version "1.2.1"
resolved "https://registry.npmjs.org/@arkecosystem/crypto-networks/-/crypto-networks-1.2.1.tgz#0715507bd0d04042ec710e09e2f51de4521de63e"
integrity sha512-Dyk9LZKzV4lS7uddjYQpeN1tS0r40i4tyWGFEipvlFEgqAECKQhH6MBnR+tWMC3c6Q+DV1EVA7F+n8KIjW4fmw==

"@arkecosystem/[email protected]":
version "1.3.1"
Expand Down

0 comments on commit bd37119

Please sign in to comment.