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

chore(core-state): remove forget methods and use index instead #3962

Merged
merged 10 commits into from
Aug 18, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ describe("Transaction Forging - Delegate Resignation", () => {

const takenDelegates = walletRepository.allByUsername().slice(0, 50);
for (const delegate of takenDelegates) {
walletRepository.forgetByUsername(delegate.getAttribute("delegate.username"));
for (const indexName of walletRepository.getIndexNames()) {
walletRepository.getIndex(indexName).forgetWallet(delegate);
}
}

// Resign a delegate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe("API 2.0 - Delegates", () => {
expect(response).toBeSuccessfulResponse();
expect(response.data.data).toBeArray();

expect(response.data.data[0].username).toBe("genesis_51");
expect(response.data.data[0].username).toBe("genesis_10");
expect(response.data.data[0].votes).toBe("0");

// Apply the original vote balances
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ describe("EntityController", () => {
id: registrationTxId,
},
};

await expect(controller.show(request, undefined)).resolves.toThrowError("Entity not found");
});

Expand All @@ -116,13 +116,13 @@ describe("EntityController", () => {
},
};

walletRepository.forgetByIndex(MagistrateIndex.Entities, registrationTxId);
walletRepository.getIndex(MagistrateIndex.Entities).forget(registrationTxId);

await expect(controller.show(request, undefined)).resolves.toThrowError("Entity not found");
});

it("should return error if wallet has not indexed entity attribute", async () => {
walletRepository.forgetByIndex(MagistrateIndex.Entities, registrationTxId);
walletRepository.getIndex(MagistrateIndex.Entities).forget(registrationTxId);

senderWallet = buildSenderWallet(app);

Expand Down
41 changes: 32 additions & 9 deletions __tests__/unit/core-state/__utils__/fixture-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,25 +34,48 @@ export class FixtureGenerator {
}

public generateHtlcLocks(): Wallet[] {
return this.genesisBlock.transactions
const wallets: Wallet[] = [];

this.genesisBlock.transactions
.filter((transaction) => transaction.recipientId)
.map((transaction, i) => {
.forEach((transaction, i) => {
const address = Identities.Address.fromPublicKey(transaction.senderPublicKey);
const wallet = new Wallet(address, new Services.Attributes.AttributeMap(this.attributeSet));
wallet.publicKey = transaction.senderPublicKey;
wallet.setAttribute("htlc.locks", {
[transaction.id]: {
let wallet = wallets.find((x) => x.address === address);

if (!wallet) {
wallet = new Wallet(address, new Services.Attributes.AttributeMap(this.attributeSet));
wallet.publicKey = transaction.senderPublicKey;
wallets.push(wallet);
}

if (wallet.hasAttribute("htlc.locks")) {
const locks: any = wallet.getAttribute("htlc.locks");

locks[transaction.id] = {
amount: Utils.BigNumber.make(10),
recipientId: transaction.recipientId,
secretHash: transaction.id,
expiration: {
type: 1,
value: 100 * (i + 1),
},
},
});
return wallet;
};
} else {
wallet.setAttribute("htlc.locks", {
[transaction.id]: {
amount: Utils.BigNumber.make(10),
recipientId: transaction.recipientId,
secretHash: transaction.id,
expiration: {
type: 1,
value: 100 * (i + 1),
},
},
});
}
});

return wallets;
}

public generateBridgeChainWallets(): Wallet[] {
Expand Down
19 changes: 12 additions & 7 deletions __tests__/unit/core-state/block-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ let spyApplyVoteBalances: jest.SpyInstance;
let spyRevertVoteBalances: jest.SpyInstance;
let spyRevertBlockFromForger: jest.SpyInstance;

const forgetWallet = (wallet: Wallet) => {
for (const indexName of walletRepo.getIndexNames()) {
const index = walletRepo.getIndex(indexName);

index.forgetWallet(wallet);
}
};

beforeAll(async () => {
const initialEnv = await setUp(setUpDefaults, true); // todo: why do I have to skip booting?
walletRepo = initialEnv.walletRepo;
Expand Down Expand Up @@ -310,11 +318,6 @@ describe("BlockState", () => {
expect(forgingWallet.balance).toEqual(balanceBefore);
});

it("should throw if there is no forger wallet", async () => {
walletRepo.forgetByPublicKey(forgingWallet.publicKey);
await expect(blockState.applyBlock(blocks[0])).toReject();
});

it("should update sender's and recipient's delegate's vote balance when applying transaction", async () => {
const sendersDelegate = forgingWallet;
sendersDelegate.setAttribute("delegate.voteBalance", Utils.BigNumber.ZERO);
Expand Down Expand Up @@ -575,14 +578,16 @@ describe("BlockState", () => {

it("not fail to apply transaction if the recipient doesn't exist", async () => {
transaction.data.recipientId = "don'tExist";
walletRepo.forgetByAddress(recipientWallet.address);

forgetWallet(recipientWallet);

await expect(blockState.applyTransaction(transaction)).toResolve();
});

it("not fail to revert transaction if the recipient doesn't exist", async () => {
transaction.data.recipientId = "don'tExist";
walletRepo.forgetByAddress(recipientWallet.address);

forgetWallet(recipientWallet);

await expect(blockState.revertTransaction(transaction)).toResolve();
});
Expand Down
102 changes: 68 additions & 34 deletions __tests__/unit/core-state/wallets/wallet-index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,47 +6,22 @@ import { setUp } from "../setup";

let factory: Factory;

let wallet: Wallets.Wallet;
let walletIndex: WalletIndex;

beforeAll(async () => {
const initialEnv = await setUp();
factory = initialEnv.factory.get("Wallet");
});

describe("WalletIndex", () => {
let wallet: Wallets.Wallet;
let walletIndex: WalletIndex;

beforeAll(() => {
wallet = factory.make<Wallets.Wallet>();
walletIndex = new WalletIndex((index, wallet) => {
index.set(wallet.address, wallet);
});
});

it("should index and forget wallets", () => {
expect(walletIndex.has(wallet.address)).toBeFalse();

walletIndex.index(wallet);
expect(walletIndex.has(wallet.address)).toBeTrue();

walletIndex.forget(wallet.address);
expect(walletIndex.has(wallet.address)).toBeFalse();
});

it("should set and get addresses", () => {
expect(walletIndex.has(wallet.address)).toBeFalse();

walletIndex.index(wallet);
walletIndex.set(wallet.address, wallet);

expect(walletIndex.get(wallet.address)).toBe(wallet);
expect(walletIndex.has(wallet.address)).toBeTrue();

expect(walletIndex.values()).toContain(wallet);

walletIndex.clear();
expect(walletIndex.has(wallet.address)).toBeFalse();
beforeEach(() => {
wallet = factory.make<Wallets.Wallet>();
walletIndex = new WalletIndex((index, wallet) => {
index.set(wallet.address, wallet);
});
});

describe("WalletIndex", () => {
it("should be cloneable", () => {
walletIndex.index(wallet);
walletIndex.set(wallet.address, wallet);
Expand All @@ -66,4 +41,63 @@ describe("WalletIndex", () => {
walletIndex.index(wallet);
expect(walletIndex.keys()).toContain(wallet.address);
});

describe("set", () => {
it("should set and get addresses", () => {
expect(walletIndex.has(wallet.address)).toBeFalse();

walletIndex.index(wallet);
walletIndex.set(wallet.address, wallet);

expect(walletIndex.get(wallet.address)).toBe(wallet);
expect(walletIndex.has(wallet.address)).toBeTrue();

expect(walletIndex.values()).toContain(wallet);

walletIndex.clear();
expect(walletIndex.has(wallet.address)).toBeFalse();
});

it("should override key with new wallet", () => {
const anotherWallet = factory.make<Wallets.Wallet>();

walletIndex.set("key1", wallet);
walletIndex.set("key1", anotherWallet);

expect(walletIndex.get("key1")).toBe(anotherWallet);

const entries = walletIndex.entries();
expect(entries.length).toEqual(1);
});
});

describe("forget", () => {
it("should index and forget wallets", () => {
expect(walletIndex.has(wallet.address)).toBeFalse();

walletIndex.index(wallet);
expect(walletIndex.has(wallet.address)).toBeTrue();

walletIndex.forget(wallet.address);
expect(walletIndex.has(wallet.address)).toBeFalse();
});

it("should not throw if key is not indexed", () => {
walletIndex.forget(wallet.address);
});
});

describe("forgetWallet", () => {
it("should forget wallet", () => {
walletIndex.index(wallet);
expect(walletIndex.get(wallet.address)).toBe(wallet);

walletIndex.forgetWallet(wallet);
expect(walletIndex.get(wallet.address)).toBeUndefined();
});

it("should not throw if wallet is not indexed", () => {
walletIndex.forgetWallet(wallet);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ describe("Wallet Repository Clone", () => {
expect(walletRepoClone.allByAddress()).toEqual([wallet]);

// TODO: similarly, this behaviour is odd - as the code hasn't been overwritten in the extended class
walletRepoClone.forgetByAddress(address);
expect(walletRepoClone.has(address)).toBeTrue();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,6 @@ describe("Wallet Repository Copy On Write", () => {
expect(walletRepoCopyOnWrite.allByAddress()).toEqual([wallet]);

// TODO: similarly, this behaviour is odd - as the code hasn't been overwritten in the extended class
walletRepoCopyOnWrite.forgetByAddress(address);
expect(walletRepoCopyOnWrite.has(address)).toBeTrue();
});

Expand Down