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

Jump to bottom

fix(ref-imp): make bitcoin logic make less calls to db #906

Merged
merged 7 commits into from
Oct 30, 2020
Merged

fix(ref-imp): make bitcoin logic make less calls to db #906

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a6576f2
fix(ref-imp): make bitcoin logic make less calls to db
isaacJChen Oct 30, 2020
983b3cf
fix(ref-imp): make bitcoin logic make less calls to db
isaacJChen Oct 30, 2020
a646001
fix(ref-imp): fix lint
isaacJChen Oct 30, 2020
eacc17a
fix(ref-imp): addess pr comments
isaacJChen Oct 30, 2020
890fcb1
fix(ref-imp): add comment
isaacJChen Oct 30, 2020
6ff727b
fix(ref-imp): add comment
isaacJChen Oct 30, 2020
6e9936a
fix(ref-imp): add comment
isaacJChen Oct 30, 2020
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
11 changes: 7 additions & 4 deletions lib/bitcoin/BitcoinProcessor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export default class BitcoinProcessor {

private bitcoinDataDirectory: string | undefined;

/** at least 10 blocks per page unless reaching the last block */
/** at least 100 blocks per page unless reaching the last block */
private static readonly pageSizeInBlocks = 100;

public constructor (config: IBitcoinConfig, versionModels: VersionModel[]) {
Expand Down Expand Up @@ -440,7 +440,7 @@ export default class BitcoinProcessor {
}

// if not enough blocks to fill the page then there are no more transactions
isaacJChen marked this conversation as resolved.
Show resolved Hide resolved
const moreTransactions = lastBlockSeen <= currentLastProcessedBlock.height;
const moreTransactions = lastBlockSeen < currentLastProcessedBlock.height;

return {
transactions,
Expand Down Expand Up @@ -807,12 +807,14 @@ export default class BitcoinProcessor {
}

/**
* Return transactions since transaction number and number of blocks acquired (Will get at least 1 block worth of data)
* Return transactions since transaction number and the last block seen
* (Will get at least 1 full block worth of data unless there is no transaction to return)
* @param since Transaction number to query since
* @param maxBlockHeight The last block height to consider included in transactions
* @returns a tuple of [transactions, lastBlockSeen]
*/
private async getTransactionsSince (since: number | undefined, maxBlockHeight: number): Promise<[TransactionModel[], number]> {
// test against undefined because 0 is falsey and this helps differenciate the behavior between 0 and undefined
let inclusiveBeginTransactionTime = since === undefined ? this.genesisBlockNumber : TransactionNumber.getBlockNumber(since);
isaacJChen marked this conversation as resolved.
Show resolved Hide resolved
isaacJChen marked this conversation as resolved.
Show resolved Hide resolved

const transactionsToReturn: TransactionModel[] = [];
Expand All @@ -834,6 +836,7 @@ export default class BitcoinProcessor {
transactionsToReturn.push(...transactions);
}

return [transactionsToReturn, inclusiveBeginTransactionTime];
// the -1 makes the last seen transaction time inclusive because the variable is set to the exclusive one every loop
return [transactionsToReturn, inclusiveBeginTransactionTime - 1];
}
}
3 changes: 2 additions & 1 deletion lib/core/versions/latest/DocumentComposer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,8 @@ export default class DocumentComposer {
const allowedProperties = new Set(['action', 'ids']);
for (const property in patch) {
if (!allowedProperties.has(property)) {
throw new SidetreeError(ErrorCode.DocumentComposerUnknownPropertyInRemovePublicKeysPatch, `Unexpected property ${property} in remove-public-keys patch.`);
throw new SidetreeError(ErrorCode.DocumentComposerUnknownPropertyInRemovePublicKeysPatch,
`Unexpected property ${property} in remove-public-keys patch.`);
}
}

Expand Down
32 changes: 16 additions & 16 deletions tests/bitcoin/BitcoinProcessor.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,13 +316,13 @@ describe('BitcoinProcessor', () => {
hash: 'some hash',
previousHash: 'previous hash'
};
const laterThanMock = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake(() => {
const getTransactionsStartingFromSpy = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake(() => {
return Promise.resolve(transactions);
});

const actual = await bitcoinProcessor.transactions();
expect(verifyMock).toHaveBeenCalledTimes(1); // called after data was retrieved
expect(laterThanMock).toHaveBeenCalled();
expect(getTransactionsStartingFromSpy).toHaveBeenCalled();
expect(actual.moreTransactions).toBeTruthy(); // true because page size is reached
expect(actual.transactions).toEqual(transactions);
done();
Expand All @@ -338,13 +338,13 @@ describe('BitcoinProcessor', () => {
hash: 'some hash',
previousHash: 'previous hash'
};
const laterThanMock = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake(() => {
const getTransactionsStartingFromSpy = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake(() => {
return Promise.resolve(transactions);
});

const actual = await bitcoinProcessor.transactions();
expect(verifyMock).toHaveBeenCalledTimes(1);
expect(laterThanMock).toHaveBeenCalled();
expect(getTransactionsStartingFromSpy).toHaveBeenCalled();
expect(actual.moreTransactions).toBeFalsy(); // don't need more transactions because page size is not reached (last block reached)
expect(actual.transactions).toEqual(transactions.slice(0, transactions.length - 1)); // the last one should be omitted because it is processing
done();
Expand All @@ -360,13 +360,13 @@ describe('BitcoinProcessor', () => {
hash: 'some hash',
previousHash: 'previous hash'
};
const laterThanMock = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake(() => {
const getTransactionsStartingFromSpy = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake(() => {
return Promise.resolve(transactions);
});

const actual = await bitcoinProcessor.transactions();
expect(verifyMock).toHaveBeenCalledTimes(1);
expect(laterThanMock).toHaveBeenCalled();
expect(getTransactionsStartingFromSpy).toHaveBeenCalled();
expect(actual.moreTransactions).toBeFalsy(); // Looks at genesis to genesis + 100 but lastProcessedBlock is genesis + 99
expect(actual.transactions).toEqual(transactions);
done();
Expand All @@ -382,13 +382,13 @@ describe('BitcoinProcessor', () => {
hash: 'some hash',
previousHash: 'previous hash'
};
const laterThanMock = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake(() => {
const getTransactionsStartingFromSpy = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake(() => {
return Promise.resolve(transactions);
});

const actual = await bitcoinProcessor.transactions();
expect(verifyMock).toHaveBeenCalledTimes(1);
expect(laterThanMock).toHaveBeenCalled();
expect(getTransactionsStartingFromSpy).toHaveBeenCalled();
expect(actual.moreTransactions).toBeFalsy(); // no more transactions because past last block
expect(actual.transactions).toEqual([]); // no return because nothing is processed
done();
Expand All @@ -403,7 +403,7 @@ describe('BitcoinProcessor', () => {
hash: 'some hash',
previousHash: 'previous hash'
};
const laterThanMock = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake((begin) => {
const getTransactionsStartingFromSpy = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake((begin) => {
if (begin === bitcoinProcessor['genesisBlockNumber'] + 500) {
return Promise.resolve(createTransactions(1, begin + 5, false));
}
Expand All @@ -412,7 +412,7 @@ describe('BitcoinProcessor', () => {

const actual = await bitcoinProcessor.transactions();
expect(verifyMock).toHaveBeenCalledTimes(1);
expect(laterThanMock).toHaveBeenCalledTimes(6);
expect(getTransactionsStartingFromSpy).toHaveBeenCalledTimes(6);
expect(actual.moreTransactions).toBeTruthy(); // more transactions because it didn't reach latestProcessedBlock
expect(actual.transactions.length).toEqual(1);
done();
Expand All @@ -425,13 +425,13 @@ describe('BitcoinProcessor', () => {
hash: 'some hash',
previousHash: 'previous hash'
};
const laterThanMock = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake(() => {
const getTransactionsStartingFromSpy = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake(() => {
return Promise.resolve([]);
});

const actual = await bitcoinProcessor.transactions();
expect(verifyMock).toHaveBeenCalledTimes(1);
expect(laterThanMock).toHaveBeenCalled();
expect(getTransactionsStartingFromSpy).toHaveBeenCalled();
expect(actual.moreTransactions).toBeFalsy();
expect(actual.transactions).toEqual([]);
done();
Expand All @@ -454,13 +454,13 @@ describe('BitcoinProcessor', () => {
return Promise.resolve(true);
});
const transactions = createTransactions(BitcoinProcessor['pageSizeInBlocks'], expectedHeight, true);
const laterThanMock = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake(() => {
const getTransactionsStartingFromSpy = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.callFake(() => {
return Promise.resolve(transactions);
});

const actual = await bitcoinProcessor.transactions(expectedTransactionNumber, expectedHash);
expect(verifyMock).toHaveBeenCalledTimes(2);
expect(laterThanMock).toHaveBeenCalled();
expect(getTransactionsStartingFromSpy).toHaveBeenCalled();
expect(actual.moreTransactions).toBeFalsy();
transactions.shift();
expect(actual.transactions).toEqual(transactions); // the first one should be excluded because of since
Expand Down Expand Up @@ -531,10 +531,10 @@ describe('BitcoinProcessor', () => {
const expectedTransactionNumber = TransactionNumber.construct(expectedHeight, 0);
const verifyMock = spyOn(bitcoinProcessor, 'verifyBlock' as any).and.returnValue(Promise.resolve(true));
const transactions = createTransactions(BitcoinProcessor['pageSizeInBlocks'], expectedHeight, true);
const laterThanMock = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.returnValue(Promise.resolve(transactions));
const getTransactionsStartingFromSpy = spyOn(bitcoinProcessor['transactionStore'], 'getTransactionsStartingFrom').and.returnValue(Promise.resolve(transactions));
const actual = await bitcoinProcessor.transactions(expectedTransactionNumber, expectedHash);
expect(verifyMock).toHaveBeenCalled();
expect(laterThanMock).toHaveBeenCalled();
expect(getTransactionsStartingFromSpy).toHaveBeenCalled();
expect(actual.moreTransactions).toBeTruthy();
done();
});
Expand Down