From cf08834da7d71f35df9ab845ef7571fbd719fe2e Mon Sep 17 00:00:00 2001 From: Will Meister Date: Tue, 9 Jul 2019 16:01:51 -0500 Subject: [PATCH 01/15] Initial commit of Aggregator StateUpdate Ingestion --- .../core/src/app/aggregator/aggregator.ts | 32 ++++ packages/core/src/app/aggregator/index.ts | 1 + packages/core/src/app/index.ts | 1 + .../types/aggregator/aggregator.interface.ts | 11 ++ .../aggregator/block-manager.interface.ts | 13 ++ packages/core/src/types/aggregator/index.ts | 2 + packages/core/src/types/index.ts | 1 + .../src/types/ovm/state-manager.interface.ts | 3 +- .../types/serialization/state.interface.ts | 5 + .../test/app/aggregator/aggregator.spec.ts | 145 ++++++++++++++++++ .../merkle-interval-tree.spec.ts | 31 +--- .../core/test/app/ovm/state-manager.spec.ts | 26 +--- packages/core/test/app/utils/test-utils.ts | 46 ++++++ 13 files changed, 270 insertions(+), 47 deletions(-) create mode 100644 packages/core/src/app/aggregator/aggregator.ts create mode 100644 packages/core/src/app/aggregator/index.ts create mode 100644 packages/core/src/types/aggregator/aggregator.interface.ts create mode 100644 packages/core/src/types/aggregator/block-manager.interface.ts create mode 100644 packages/core/src/types/aggregator/index.ts create mode 100644 packages/core/test/app/aggregator/aggregator.spec.ts create mode 100644 packages/core/test/app/utils/test-utils.ts diff --git a/packages/core/src/app/aggregator/aggregator.ts b/packages/core/src/app/aggregator/aggregator.ts new file mode 100644 index 00000000..51eeabab --- /dev/null +++ b/packages/core/src/app/aggregator/aggregator.ts @@ -0,0 +1,32 @@ +import BigNum = require('bn.js') + +import { Aggregator, BlockManager } from '../../types/aggregator' +import { StateManager } from '../../types/ovm' +import { Transaction, TransactionResult } from '../../types/serialization' + +export class DefaultAggregator implements Aggregator { + private readonly stateManager: StateManager + private readonly blockManager: BlockManager + + public constructor(stateManager: StateManager, blockManager: BlockManager) { + this.stateManager = stateManager + this.blockManager = blockManager + } + + public async ingestTransaction( + transaction: Transaction, + witness: string + ): Promise { + const blockNum: BigNum = await this.blockManager.getNextBlockNumber() + + const { + stateUpdate, + }: TransactionResult = await this.stateManager.executeTransaction( + transaction, + blockNum, + witness + ) + + await this.blockManager.addPendingStateUpdate(stateUpdate) + } +} diff --git a/packages/core/src/app/aggregator/index.ts b/packages/core/src/app/aggregator/index.ts new file mode 100644 index 00000000..4e1b7202 --- /dev/null +++ b/packages/core/src/app/aggregator/index.ts @@ -0,0 +1 @@ +export * from './aggregator' diff --git a/packages/core/src/app/index.ts b/packages/core/src/app/index.ts index 5fecd760..9f1f1ef5 100644 --- a/packages/core/src/app/index.ts +++ b/packages/core/src/app/index.ts @@ -1,3 +1,4 @@ +export * from './aggregator' export * from './ovm' export * from './keystore' export * from './block-production' diff --git a/packages/core/src/types/aggregator/aggregator.interface.ts b/packages/core/src/types/aggregator/aggregator.interface.ts new file mode 100644 index 00000000..388584f0 --- /dev/null +++ b/packages/core/src/types/aggregator/aggregator.interface.ts @@ -0,0 +1,11 @@ +import { Transaction } from '../serialization' + +export interface Aggregator { + /** + * Notifies the Aggregator of the provided Transaction so it may be included in the next block. + * + * @param transaction the Transaction in question + * @param witness the Witness of the Transaction in question + */ + ingestTransaction(transaction: Transaction, witness: string): Promise +} diff --git a/packages/core/src/types/aggregator/block-manager.interface.ts b/packages/core/src/types/aggregator/block-manager.interface.ts new file mode 100644 index 00000000..33f99e97 --- /dev/null +++ b/packages/core/src/types/aggregator/block-manager.interface.ts @@ -0,0 +1,13 @@ +import BigNum = require('bn.js') + +import { StateUpdate } from '../serialization' + +/** + * Block Manager wrapping Block CRUD operations. + */ +export interface BlockManager { + getNextBlockNumber(): Promise + addPendingStateUpdate(stateUpdate: StateUpdate): Promise + getPendingStateUpdates(): Promise + submitNextBlock(): Promise +} diff --git a/packages/core/src/types/aggregator/index.ts b/packages/core/src/types/aggregator/index.ts new file mode 100644 index 00000000..7938b439 --- /dev/null +++ b/packages/core/src/types/aggregator/index.ts @@ -0,0 +1,2 @@ +export * from './aggregator.interface' +export * from './block-manager.interface' diff --git a/packages/core/src/types/index.ts b/packages/core/src/types/index.ts index 27d14cd3..7638e2ff 100644 --- a/packages/core/src/types/index.ts +++ b/packages/core/src/types/index.ts @@ -1,3 +1,4 @@ +export * from './aggregator' export * from './ovm' export * from './miscellaneous' export * from './keystore' diff --git a/packages/core/src/types/ovm/state-manager.interface.ts b/packages/core/src/types/ovm/state-manager.interface.ts index 2f423480..d76a4376 100644 --- a/packages/core/src/types/ovm/state-manager.interface.ts +++ b/packages/core/src/types/ovm/state-manager.interface.ts @@ -7,6 +7,7 @@ import { StateQueryResult, StateUpdate, Transaction, + TransactionResult, } from '../../types' export interface StateManager { @@ -22,7 +23,7 @@ export interface StateManager { transaction: Transaction, inBlock: BigNum, witness: string - ): Promise<{ stateUpdate: StateUpdate; validRanges: Range[] }> + ): Promise /** * Validates the provided HistoryProof. diff --git a/packages/core/src/types/serialization/state.interface.ts b/packages/core/src/types/serialization/state.interface.ts index 9298dd72..583ecfdf 100644 --- a/packages/core/src/types/serialization/state.interface.ts +++ b/packages/core/src/types/serialization/state.interface.ts @@ -44,6 +44,11 @@ export interface Transaction { body: any } +export interface TransactionResult { + stateUpdate: StateUpdate + validRanges: Range[] +} + export interface OwnershipBody { newState: StateObject originBlock: BigNum diff --git a/packages/core/test/app/aggregator/aggregator.spec.ts b/packages/core/test/app/aggregator/aggregator.spec.ts new file mode 100644 index 00000000..087b1784 --- /dev/null +++ b/packages/core/test/app/aggregator/aggregator.spec.ts @@ -0,0 +1,145 @@ +import BigNum = require('bn.js') +import { should } from '../../setup' + +import { + Aggregator, + BlockManager, + HistoryProof, + StateManager, + StateQuery, + StateQueryResult, + StateUpdate, + Transaction, + TransactionResult, +} from '../../../src/types' +import { DefaultAggregator, ONE, ZERO } from '../../../src/app/' +import { TestUtils } from '../utils/test-utils' +import * as assert from 'assert' + +/******************* + * Mocks & Helpers * + *******************/ + +class DummyBlockManager implements BlockManager { + private nextBlockNumber: BigNum + private readonly stateUpdates: StateUpdate[] + + constructor() { + this.nextBlockNumber = ONE + this.stateUpdates = [] + } + + public async addPendingStateUpdate(stateUpdate: StateUpdate): Promise { + this.stateUpdates.push(stateUpdate) + } + + public async getNextBlockNumber(): Promise { + return this.nextBlockNumber + } + + public async getPendingStateUpdates(): Promise { + return this.stateUpdates + } + + public async submitNextBlock(): Promise { + this.stateUpdates.length = 0 + this.nextBlockNumber = this.nextBlockNumber.add(ONE) + } +} + +class DummyStateManager implements StateManager { + private throwOnExecute: boolean = false + private executeTransactionResults: TransactionResult[] + + public setExecuteTransactionResults( + transactionResults: TransactionResult[] + ): void { + this.executeTransactionResults = transactionResults + } + + public throwOnExecuteTransaction(): void { + this.throwOnExecute = true + } + + public async executeTransaction( + transaction: Transaction, + inBlock: BigNum, + witness: string + ): Promise { + if (this.throwOnExecute) { + this.throwOnExecute = false + throw Error('I was configured to throw') + } + return this.executeTransactionResults.shift() + } + + public async ingestHistoryProof(historyProof: HistoryProof): Promise { + return undefined + } + + public async queryState(query: StateQuery): Promise { + return undefined + } +} + +/********* + * TESTS * + *********/ + +describe('DefaultAggregator', () => { + describe('ingestTransaction', () => { + it('Ingests transaction correctly', async () => { + const numTransactions: number = 5 + const transactionResults: TransactionResult[] = TestUtils.generateNSequentialTransactionResults( + numTransactions + ) + + const blockManager: DummyBlockManager = new DummyBlockManager() + const stateManager: DummyStateManager = new DummyStateManager() + stateManager.setExecuteTransactionResults([...transactionResults]) + + const aggregator: Aggregator = new DefaultAggregator( + stateManager, + blockManager + ) + + const transaction: Transaction = { + depositAddress: '', + range: { + start: ZERO, + end: ONE, + }, + body: {}, + } + + for (let i = 0; i < numTransactions; i++) { + await aggregator.ingestTransaction(transaction, '') + } + + const stateUpdates: StateUpdate[] = await blockManager.getPendingStateUpdates() + + stateUpdates.length.should.equal(numTransactions) + for (let i = 0; i < numTransactions; i++) { + stateUpdates[i].should.equal(transactionResults[i].stateUpdate) + } + }) + + it('Throws if executeTransaction throws', async () => { + const blockManager: DummyBlockManager = new DummyBlockManager() + const stateManager: DummyStateManager = new DummyStateManager() + stateManager.throwOnExecuteTransaction() + + const aggregator: Aggregator = new DefaultAggregator( + stateManager, + blockManager + ) + + try { + await aggregator.ingestTransaction(undefined, '') + assert(false, 'This should have thrown') + } catch (e) { + // This is success + } + }) + }) +}) diff --git a/packages/core/test/app/block-production/merkle-interval-tree.spec.ts b/packages/core/test/app/block-production/merkle-interval-tree.spec.ts index 9c9c88ab..43863d9e 100644 --- a/packages/core/test/app/block-production/merkle-interval-tree.spec.ts +++ b/packages/core/test/app/block-production/merkle-interval-tree.spec.ts @@ -8,34 +8,13 @@ import BigNum = require('bn.js') /* Internal Imports */ import { AbiStateUpdate, - AbiStateObject, AbiRange, GenericMerkleIntervalTree, GenericMerkleIntervalTreeNode, MerkleStateIntervalTree, PlasmaBlock, } from '../../../src/app/' - -function generateNSequentialStateUpdates( - numerOfUpdates: number -): AbiStateUpdate[] { - const stateUpdates: AbiStateUpdate[] = [] - for (let i = 0; i < numerOfUpdates; i++) { - const stateObject = new AbiStateObject( - '0xbdAd2846585129Fc98538ce21cfcED21dDDE0a63', - '0x123456' - ) - const range = new AbiRange(new BigNum(i * 100), new BigNum((i + 0.5) * 100)) - const stateUpdate = new AbiStateUpdate( - stateObject, - range, - new BigNum(1), - '0xbdAd2846585129Fc98538ce21cfcED21dDDE0a63' - ) - stateUpdates.push(stateUpdate) - } - return stateUpdates -} +import { TestUtils } from '../utils/test-utils' describe('Interval Trees and Plasma Blocks', () => { describe('GenericMerkleIntervalTreeNode', () => { @@ -117,13 +96,13 @@ describe('Interval Trees and Plasma Blocks', () => { }) describe('MerkleStateIntervalTree', () => { it("'new MerkleStateIntervalTree' generate a tree without throwing", async () => { - const stateUpdates = generateNSequentialStateUpdates(4) + const stateUpdates = TestUtils.generateNSequentialStateUpdates(4) const merkleStateIntervalTree = new MerkleStateIntervalTree(stateUpdates) log('root', merkleStateIntervalTree.root()) }) it("'MerkleStateIntervalTree.verifyExectedRoot' should throw if state update range intersects branch bounds", async () => { // generate some valid tree contents - const stateUpdates = generateNSequentialStateUpdates(5) + const stateUpdates = TestUtils.generateNSequentialStateUpdates(5) // make an invalid range intersecting the second SU const faultyUpdateIndex = 0 const updateToReplace = stateUpdates[faultyUpdateIndex] @@ -159,7 +138,7 @@ describe('Interval Trees and Plasma Blocks', () => { }) describe('PlasmaBlock', () => { it('should generate a plasma block without throwing', async () => { - const stateUpdates = generateNSequentialStateUpdates(4) + const stateUpdates = TestUtils.generateNSequentialStateUpdates(4) const blockContents = [ { assetId: Buffer.from( @@ -180,7 +159,7 @@ describe('Interval Trees and Plasma Blocks', () => { log(plasmaBlock) }) it('should generate and verify a StateUpdateInclusionProof', async () => { - const stateUpdates = generateNSequentialStateUpdates(4) + const stateUpdates = TestUtils.generateNSequentialStateUpdates(4) const blockContents = [ { assetId: Buffer.from( diff --git a/packages/core/test/app/ovm/state-manager.spec.ts b/packages/core/test/app/ovm/state-manager.spec.ts index 56c5f7e4..65dfb461 100644 --- a/packages/core/test/app/ovm/state-manager.spec.ts +++ b/packages/core/test/app/ovm/state-manager.spec.ts @@ -19,6 +19,7 @@ import { StateObject, StateUpdate, Transaction, + TransactionResult, VerifiedStateUpdate, } from '../../../src/types' import * as assert from 'assert' @@ -202,10 +203,7 @@ describe('DefaultStateManager', () => { pluginManager ) - const result: { - stateUpdate: StateUpdate - validRanges: Range[] - } = await stateManager.executeTransaction( + const result: TransactionResult = await stateManager.executeTransaction( transaction, nextBlockNumber, defaultWitness @@ -251,10 +249,7 @@ describe('DefaultStateManager', () => { pluginManager ) - const result: { - stateUpdate: StateUpdate - validRanges: Range[] - } = await stateManager.executeTransaction( + const result: TransactionResult = await stateManager.executeTransaction( transaction, nextBlockNumber, defaultWitness @@ -302,10 +297,7 @@ describe('DefaultStateManager', () => { pluginManager ) - const result: { - stateUpdate: StateUpdate - validRanges: Range[] - } = await stateManager.executeTransaction( + const result: TransactionResult = await stateManager.executeTransaction( transaction, nextBlockNumber, defaultWitness @@ -351,10 +343,7 @@ describe('DefaultStateManager', () => { pluginManager ) - const result: { - stateUpdate: StateUpdate - validRanges: Range[] - } = await stateManager.executeTransaction( + const result: TransactionResult = await stateManager.executeTransaction( transaction, nextBlockNumber, defaultWitness @@ -384,10 +373,7 @@ describe('DefaultStateManager', () => { pluginManager ) - const result: { - stateUpdate: StateUpdate - validRanges: Range[] - } = await stateManager.executeTransaction( + const result: TransactionResult = await stateManager.executeTransaction( transaction, nextBlockNumber, defaultWitness diff --git a/packages/core/test/app/utils/test-utils.ts b/packages/core/test/app/utils/test-utils.ts new file mode 100644 index 00000000..8ea31867 --- /dev/null +++ b/packages/core/test/app/utils/test-utils.ts @@ -0,0 +1,46 @@ +import BigNum = require('bn.js') +import { + AbiRange, + AbiStateObject, + AbiStateUpdate, +} from '../../../src/app/serialization' +import { TransactionResult } from '../../../src/types/serialization' + +export class TestUtils { + public static generateNSequentialStateUpdates( + numberOfUpdates: number + ): AbiStateUpdate[] { + const stateUpdates: AbiStateUpdate[] = [] + for (let i = 0; i < numberOfUpdates; i++) { + const stateObject = new AbiStateObject( + '0xbdAd2846585129Fc98538ce21cfcED21dDDE0a63', + '0x123456' + ) + const range = new AbiRange( + new BigNum(i * 100), + new BigNum((i + 0.5) * 100) + ) + const stateUpdate = new AbiStateUpdate( + stateObject, + range, + new BigNum(1), + '0xbdAd2846585129Fc98538ce21cfcED21dDDE0a63' + ) + stateUpdates.push(stateUpdate) + } + return stateUpdates + } + + public static generateNSequentialTransactionResults( + numberofUpdates: number + ): TransactionResult[] { + return this.generateNSequentialStateUpdates(numberofUpdates).map( + (abiStateUpdate: AbiStateUpdate): TransactionResult => { + return { + stateUpdate: abiStateUpdate, + validRanges: [abiStateUpdate.range], + } + } + ) + } +} From b1b779148d15a6a318ee2115c3763865a855725f Mon Sep 17 00:00:00 2001 From: Will Meister Date: Wed, 10 Jul 2019 12:54:02 -0500 Subject: [PATCH 02/15] Adds: - BlockTransactionCommitment returned from ingestTransaction(...) - Ability to get Public Key of Aggregator to validate signed BlockTransaction - Some crypto utils - Range utilities Improves: - Tests for Aggregator, testing return type, range validity, and signature - Crypto verifySignature stub --- .../core/src/app/aggregator/aggregator.ts | 39 ++++++++- packages/core/src/app/utils/crypto.ts | 29 ++++++- packages/core/src/app/utils/equals.ts | 33 +++++++- packages/core/src/app/utils/range.ts | 33 ++++++++ .../types/aggregator/aggregator.interface.ts | 16 +++- .../types/ovm/predicate-plugin.interface.ts | 2 +- .../types/serialization/state.interface.ts | 10 +++ .../test/app/aggregator/aggregator.spec.ts | 82 ++++++++++++++++--- .../src/plugins/ownership-predicate.ts | 2 +- .../test/ownership-predicate.spec.ts | 7 +- 10 files changed, 228 insertions(+), 25 deletions(-) diff --git a/packages/core/src/app/aggregator/aggregator.ts b/packages/core/src/app/aggregator/aggregator.ts index 51eeabab..686e14d3 100644 --- a/packages/core/src/app/aggregator/aggregator.ts +++ b/packages/core/src/app/aggregator/aggregator.ts @@ -2,9 +2,19 @@ import BigNum = require('bn.js') import { Aggregator, BlockManager } from '../../types/aggregator' import { StateManager } from '../../types/ovm' -import { Transaction, TransactionResult } from '../../types/serialization' +import { + BlockTransaction, + BlockTransactionCommitment, + Transaction, + TransactionResult, +} from '../../types/serialization' +import { rangesSpanRange, sign } from '../utils' export class DefaultAggregator implements Aggregator { + private readonly publicKey: string = + 'TODO: figure out public key storage and access' + private readonly privateKey: string = + 'TODO: figure out private key storage and access' private readonly stateManager: StateManager private readonly blockManager: BlockManager @@ -16,17 +26,38 @@ export class DefaultAggregator implements Aggregator { public async ingestTransaction( transaction: Transaction, witness: string - ): Promise { - const blockNum: BigNum = await this.blockManager.getNextBlockNumber() + ): Promise { + const blockNumber: BigNum = await this.blockManager.getNextBlockNumber() const { stateUpdate, + validRanges, }: TransactionResult = await this.stateManager.executeTransaction( transaction, - blockNum, + blockNumber, witness ) + if (!rangesSpanRange(validRanges, transaction.range)) { + throw Error( + 'Cannot ingest Transaction that is not valid across its entire range.' + ) + } + await this.blockManager.addPendingStateUpdate(stateUpdate) + + const blockTransaction: BlockTransaction = { + blockNumber, + transaction, + } + + return { + blockTransaction, + witness: sign(this.privateKey, blockTransaction), + } + } + + public async getPublicKey(): Promise { + return this.publicKey } } diff --git a/packages/core/src/app/utils/crypto.ts b/packages/core/src/app/utils/crypto.ts index b04f8181..148403f3 100644 --- a/packages/core/src/app/utils/crypto.ts +++ b/packages/core/src/app/utils/crypto.ts @@ -13,5 +13,32 @@ export const verifySignature = ( publicKey: any ): boolean => { // TODO: Make this do actual signature checking - return signature === publicKey + return signature === message +} + +/** + * Signs the provided message with the provided key + * + * @param key the key with which the message should be signed + * @param message the message to be signed + * + * @returns the signed message + */ +export const sign = (key: any, message: any): any => { + // TODO: Actually sign + return message +} + +/** + * Decrypts the provided encrypted message with the provided public key + * + * @param publickey the public key in question + * @param encryptedMessage the encrypted message to decrypt + */ +export const decryptWithPublicKey = ( + publickey: any, + encryptedMessage: any +): any => { + // TODO: Actually decrypt + return encryptedMessage } diff --git a/packages/core/src/app/utils/equals.ts b/packages/core/src/app/utils/equals.ts index 7b16e75f..d54b644e 100644 --- a/packages/core/src/app/utils/equals.ts +++ b/packages/core/src/app/utils/equals.ts @@ -1,5 +1,11 @@ import { Range } from '../../types/db' -import { StateObject, StateUpdate } from '../../types/serialization' +import { + BlockTransaction, + StateObject, + StateUpdate, + Transaction, +} from '../../types/serialization' +import { unwatchFile } from 'fs' /** * All of the below functions check whether or not the two provided objects are equal, @@ -68,3 +74,28 @@ export const stateUpdatesEqual = ( stateObjectsEqual(stateUpdate1.stateObject, stateUpdate2.stateObject) ) } + +export const transactionsEqual = ( + tx1: Transaction, + tx2: Transaction +): boolean => { + return ( + tx1 !== undefined && + tx2 !== undefined && + tx1.depositAddress === tx2.depositAddress && + rangesEqual(tx1.range, tx2.range) && + objectsEqual(tx1.body, tx2.body) + ) +} + +export const blockTransactionsEqual = ( + blockTx1: BlockTransaction, + blockTx2: BlockTransaction +): boolean => { + return ( + blockTx1 !== undefined && + blockTx2 !== undefined && + blockTx1.blockNumber.eq(blockTx2.blockNumber) && + transactionsEqual(blockTx1.transaction, blockTx2.transaction) + ) +} diff --git a/packages/core/src/app/utils/range.ts b/packages/core/src/app/utils/range.ts index e3bb8f92..8d530023 100644 --- a/packages/core/src/app/utils/range.ts +++ b/packages/core/src/app/utils/range.ts @@ -65,6 +65,39 @@ export const isRangeSubset = (subset: Range, superset: Range): boolean => { ) } +/** + * Determines whether the provided Ranges collectively span the Range in question + * + * @param ranges the Ranges that will/won't span the rangeToSpan + * @param rangeToSpan the Range being spanned + * @returns true if ranges span rangeToSpan, false otherwise + */ +export const rangesSpanRange = ( + ranges: Range[], + rangeToSpan: Range +): boolean => { + const sortedRanges: Range[] = ranges.sort((a: Range, b: Range) => { + return a.start.lt(b.start) ? -1 : a.start.eq(b.start) ? 0 : 1 + }) + + let spannedEnd: BigNum = rangeToSpan.start + for (const rangeElem of sortedRanges) { + // If our lowest range start is greater than our spannedEnd, the range cannot be spanned + if (rangeElem.start.gt(spannedEnd)) { + return false + } + + spannedEnd = rangeElem.end + + // If the entire range has been spanned we can , + if (spannedEnd.gte(rangeToSpan.end)) { + return true + } + } + + return false +} + /** * RangeStore makes it easy to store ranges. * When ranges are added, only the sections with diff --git a/packages/core/src/types/aggregator/aggregator.interface.ts b/packages/core/src/types/aggregator/aggregator.interface.ts index 388584f0..dc2338d4 100644 --- a/packages/core/src/types/aggregator/aggregator.interface.ts +++ b/packages/core/src/types/aggregator/aggregator.interface.ts @@ -1,4 +1,4 @@ -import { Transaction } from '../serialization' +import { BlockTransactionCommitment, Transaction } from '../serialization' export interface Aggregator { /** @@ -6,6 +6,18 @@ export interface Aggregator { * * @param transaction the Transaction in question * @param witness the Witness of the Transaction in question + * + * @returns the BlockTransactionCommitment indicating the transaction will be included in the next block + */ + ingestTransaction( + transaction: Transaction, + witness: string + ): Promise + + /** + * Gets the public key of the Aggregator to be able to validate signatures + * + * @returns the public key */ - ingestTransaction(transaction: Transaction, witness: string): Promise + getPublicKey(): Promise } diff --git a/packages/core/src/types/ovm/predicate-plugin.interface.ts b/packages/core/src/types/ovm/predicate-plugin.interface.ts index be3569fe..61105e00 100644 --- a/packages/core/src/types/ovm/predicate-plugin.interface.ts +++ b/packages/core/src/types/ovm/predicate-plugin.interface.ts @@ -13,7 +13,7 @@ export interface PredicatePlugin { executeStateTransition( previousStateUpdate: StateUpdate, transaction: Transaction, - witness: string + witness: any ): Promise // TODO: Add other methods when used diff --git a/packages/core/src/types/serialization/state.interface.ts b/packages/core/src/types/serialization/state.interface.ts index 583ecfdf..aeed85b6 100644 --- a/packages/core/src/types/serialization/state.interface.ts +++ b/packages/core/src/types/serialization/state.interface.ts @@ -49,6 +49,16 @@ export interface TransactionResult { validRanges: Range[] } +export interface BlockTransaction { + blockNumber: BigNum + transaction: Transaction +} + +export interface BlockTransactionCommitment { + blockTransaction: BlockTransaction + witness: any +} + export interface OwnershipBody { newState: StateObject originBlock: BigNum diff --git a/packages/core/test/app/aggregator/aggregator.spec.ts b/packages/core/test/app/aggregator/aggregator.spec.ts index 087b1784..857c3f54 100644 --- a/packages/core/test/app/aggregator/aggregator.spec.ts +++ b/packages/core/test/app/aggregator/aggregator.spec.ts @@ -4,6 +4,8 @@ import { should } from '../../setup' import { Aggregator, BlockManager, + BlockTransaction, + BlockTransactionCommitment, HistoryProof, StateManager, StateQuery, @@ -12,7 +14,14 @@ import { Transaction, TransactionResult, } from '../../../src/types' -import { DefaultAggregator, ONE, ZERO } from '../../../src/app/' +import { + blockTransactionsEqual, + decryptWithPublicKey, + DefaultAggregator, + ONE, + transactionsEqual, + ZERO, +} from '../../../src/app/' import { TestUtils } from '../utils/test-utils' import * as assert from 'assert' @@ -103,17 +112,39 @@ describe('DefaultAggregator', () => { blockManager ) - const transaction: Transaction = { - depositAddress: '', - range: { - start: ZERO, - end: ONE, - }, - body: {}, - } + const transactions: Transaction[] = [] + transactionResults.forEach((result: TransactionResult) => { + transactions.push({ + depositAddress: '', + range: result.validRanges[0], + body: {}, + }) + }) for (let i = 0; i < numTransactions; i++) { - await aggregator.ingestTransaction(transaction, '') + const txCommitment: BlockTransactionCommitment = await aggregator.ingestTransaction( + transactions[i], + '' + ) + assert( + transactionsEqual( + txCommitment.blockTransaction.transaction, + transactions[i] + ), + 'Resulting BlockTransactionCommitment does not match passed in Transaction.' + ) + + const decryptedBlockTransaction: BlockTransaction = decryptWithPublicKey( + aggregator.getPublicKey(), + txCommitment.witness + ) + assert( + blockTransactionsEqual( + decryptedBlockTransaction, + txCommitment.blockTransaction + ), + 'BlockTransactionCommitment signature is invalid' + ) } const stateUpdates: StateUpdate[] = await blockManager.getPendingStateUpdates() @@ -141,5 +172,36 @@ describe('DefaultAggregator', () => { // This is success } }) + + it('Throws if Transaction range is not valid', async () => { + const transactionResult: TransactionResult = TestUtils.generateNSequentialTransactionResults( + 1 + )[0] + + const blockManager: DummyBlockManager = new DummyBlockManager() + const stateManager: DummyStateManager = new DummyStateManager() + stateManager.setExecuteTransactionResults([transactionResult]) + + const aggregator: Aggregator = new DefaultAggregator( + stateManager, + blockManager + ) + + const transaction: Transaction = { + depositAddress: '', + range: { + start: transactionResult.validRanges[0].start, + end: transactionResult.validRanges[0].end.add(ONE), + }, + body: {}, + } + + try { + await aggregator.ingestTransaction(transaction, '') + assert(false, 'This should have thrown') + } catch (e) { + // This is success + } + }) }) }) diff --git a/packages/predicates/src/plugins/ownership-predicate.ts b/packages/predicates/src/plugins/ownership-predicate.ts index 0f15c312..8fc9ac60 100644 --- a/packages/predicates/src/plugins/ownership-predicate.ts +++ b/packages/predicates/src/plugins/ownership-predicate.ts @@ -10,7 +10,7 @@ export class OwnershipPredicatePlugin implements PredicatePlugin { public async executeStateTransition( previousStateUpdate: StateUpdate, transaction: Transaction, - witness: string + witness: any ): Promise { await this.validateStateTransition( previousStateUpdate, diff --git a/packages/predicates/test/ownership-predicate.spec.ts b/packages/predicates/test/ownership-predicate.spec.ts index e7f125bc..afb124cd 100644 --- a/packages/predicates/test/ownership-predicate.spec.ts +++ b/packages/predicates/test/ownership-predicate.spec.ts @@ -22,9 +22,6 @@ const defaultPredicateAddress: string = '0x123456789abcdef' const defaultOwner: string = '0x999999999999999' const newOwner: string = '0x8888888888888' -// TODO: Change this when we are actually checking signatures -const defaultWitness: string = defaultOwner - const defaultInBlock: BigNum = new BigNum(2) const defaultOriginBlock: BigNum = defaultInBlock const defaultMaxBlock: BigNum = new BigNum(10) @@ -126,7 +123,7 @@ describe('OwnershipPredicate', async () => { const stateObject: StateObject = await ownershipPredicate.executeStateTransition( defaultPreviousStateUpdate, defaultTransaction, - defaultWitness + defaultTransaction ) assert(stateObjectsEqual(stateObject, defaultNewState)) @@ -155,7 +152,7 @@ describe('OwnershipPredicate', async () => { await ownershipPredicate.executeStateTransition( defaultPreviousStateUpdate, transaction, - defaultWitness + transaction ) assert(false, 'Should have thrown an error and not gotten here') } catch (e) { From e7117ff04177a2c5ab823cee39ead04496adbb78 Mon Sep 17 00:00:00 2001 From: Will Meister Date: Wed, 10 Jul 2019 15:36:26 -0500 Subject: [PATCH 03/15] - Added BlockManager, Block DB CommitmentContract, and probably other stuff copied and adapted from https://github.com/plasma-group/pigi/pull/280 - Removed redundant constructor assigment of members - Adapted RangeBucket to fit BlockDB usage --- .../core/src/app/aggregator/aggregator.ts | 13 +- .../core/src/app/block-production/block-db.ts | 145 ++++++++++++++++++ .../src/app/block-production/block-manager.ts | 59 +++++++ .../core/src/app/block-production/index.ts | 2 + .../app/block-production/plasma-block-tree.ts | 2 +- packages/core/src/app/db/range-bucket.ts | 5 + packages/core/src/app/ovm/state-manager.ts | 11 +- packages/core/src/app/utils/numbers.ts | 1 + packages/core/src/types/aggregator/index.ts | 1 - .../block-production/block-db.interface.ts | 11 ++ .../block-manager.interface.ts | 0 .../commitment-contract.interface.ts | 3 + .../core/src/types/block-production/index.ts | 3 + .../core/src/types/db/range-db.interface.ts | 8 + .../test/app/aggregator/aggregator.spec.ts | 2 +- 15 files changed, 249 insertions(+), 17 deletions(-) create mode 100644 packages/core/src/app/block-production/block-db.ts create mode 100644 packages/core/src/app/block-production/block-manager.ts create mode 100644 packages/core/src/types/block-production/block-db.interface.ts rename packages/core/src/types/{aggregator => block-production}/block-manager.interface.ts (100%) create mode 100644 packages/core/src/types/block-production/commitment-contract.interface.ts create mode 100644 packages/core/src/types/block-production/index.ts diff --git a/packages/core/src/app/aggregator/aggregator.ts b/packages/core/src/app/aggregator/aggregator.ts index 686e14d3..d100804d 100644 --- a/packages/core/src/app/aggregator/aggregator.ts +++ b/packages/core/src/app/aggregator/aggregator.ts @@ -1,6 +1,6 @@ import BigNum = require('bn.js') -import { Aggregator, BlockManager } from '../../types/aggregator' +import { Aggregator } from '../../types/aggregator' import { StateManager } from '../../types/ovm' import { BlockTransaction, @@ -9,19 +9,18 @@ import { TransactionResult, } from '../../types/serialization' import { rangesSpanRange, sign } from '../utils' +import { BlockManager } from '../../types/block-production' export class DefaultAggregator implements Aggregator { private readonly publicKey: string = 'TODO: figure out public key storage and access' private readonly privateKey: string = 'TODO: figure out private key storage and access' - private readonly stateManager: StateManager - private readonly blockManager: BlockManager - public constructor(stateManager: StateManager, blockManager: BlockManager) { - this.stateManager = stateManager - this.blockManager = blockManager - } + public constructor( + private readonly stateManager: StateManager, + private readonly blockManager: BlockManager + ) {} public async ingestTransaction( transaction: Transaction, diff --git a/packages/core/src/app/block-production/block-db.ts b/packages/core/src/app/block-production/block-db.ts new file mode 100644 index 00000000..e774bbc5 --- /dev/null +++ b/packages/core/src/app/block-production/block-db.ts @@ -0,0 +1,145 @@ +/* External Imports */ +import BigNum = require('bn.js') +import { BaseKey, BaseRangeBucket } from '../db' +import { BlockDB } from '../../types/block-production' +import { KeyValueStore, RangeStore } from '../../types/db' +import { StateUpdate } from '../../types/serialization' +import { MAX_BIG_NUM, ONE, ZERO } from '../utils' +import { GenericMerkleIntervalTree } from './merkle-interval-tree' + +const PREFIXES = { + VARS: new BaseKey('v').encode(), + BLOCKS: new BaseKey('b').encode(), +} + +const KEYS = { + NEXT_BLOCK: Buffer.from('nextblock'), + BLOCK: new BaseKey('b', ['uint32']), +} + +/** + * Simple BlockDB implementation. + */ +export class DefaultBlockDB implements BlockDB { + private vars: KeyValueStore + private blocks: KeyValueStore + + /** + * Initializes the database wrapper. + * @param db Database to store values in. + */ + constructor(private db: KeyValueStore) { + this.vars = this.db.bucket(PREFIXES.VARS) + this.blocks = this.db.bucket(PREFIXES.BLOCKS) + } + + /** + * @returns the next plasma block number. + */ + public async getNextBlockNumber(): Promise { + // NOTE: You could theoretically cache the next block number as to avoid + // having to read from the database repeatedly. However, this introduces + // the need to ensure that the cached value and stored value never fall out + // of sync. It's probably better to hold off on implementing caching until + // this becomes a performance bottleneck. + + const buf = await this.vars.get(KEYS.NEXT_BLOCK) + // TODO: Node 12 has Buffer.readBigInt64BE(...) -- upgrade? + return new BigNum(buf.readUInt32BE(0)) + } + + /** + * Adds a state update to the list of updates to be published in the next + * plasma block. + * @param stateUpdate State update to publish in the next block. + * @returns a promise that resolves once the update has been added. + */ + public async addPendingStateUpdate(stateUpdate: StateUpdate): Promise { + const block = await this.getNextBlockStore() + const start = stateUpdate.range.start + const end = stateUpdate.range.end + + // TODO: Figure out how to implement locking here so two state updates + // can't be added at the same time. + + if (await block.hasDataInRange(start, end)) { + throw new Error('Block already contains a state update over that range.') + } + + const value = Buffer.from(JSON.stringify(stateUpdate)) + await block.put(start, end, value) + } + + /** + * @returns the list of state updates waiting to be published in the next + * plasma block. + */ + public async getPendingStateUpdates(): Promise { + const blockNumber = await this.getNextBlockNumber() + return this.getStateUpdates(blockNumber) + } + + /** + * Computes the Merkle Interval Tree root of a given block. + * @param blockNumber Block to compute a root for. + * @returns the root of the block. + */ + public async getMerkleRoot(blockNumber: BigNum): Promise { + const stateUpdates = await this.getStateUpdates(blockNumber) + + const leaves = stateUpdates.map((stateUpdate) => { + // TODO: Actually encode this. + const encodedStateUpdate = JSON.stringify(stateUpdate) + return { + start: stateUpdate.range.start, + end: stateUpdate.range.end, + data: encodedStateUpdate, + } + }) + const tree = new GenericMerkleIntervalTree(leaves) + return tree.root().hash + } + + /** + * Finalizes the next plasma block so that it can be published. + */ + public async finalizeNextBlock(): Promise { + // TODO: manage concurrency here + const prevBlockNumber = await this.getNextBlockNumber() + const nextBlockNumber = Buffer.allocUnsafe(4) + nextBlockNumber.writeUInt32BE(prevBlockNumber.add(ONE).toNumber(), 0) + await this.vars.put(KEYS.NEXT_BLOCK, nextBlockNumber) + } + + /** + * Opens the RangeDB for a specific block. + * @param blockNumber Block to open the RangeDB for. + * @returns the RangeDB instance for the given block. + */ + private async getBlockStore(blockNumber: BigNum): Promise { + const key = KEYS.BLOCK.encode([blockNumber]) + const bucket = this.blocks.bucket(key) + return new BaseRangeBucket(bucket.db, bucket.prefix) + } + + /** + * @returns the RangeDB instance for the next block to be published. + */ + private async getNextBlockStore(): Promise { + const blockNumber = await this.getNextBlockNumber() + return this.getBlockStore(blockNumber) + } + + /** + * Queries all of the state updates within a given block. + * @param blockNumber Block to query state updates for. + * @returns the list of state updates for that block. + */ + private async getStateUpdates(blockNumber: BigNum): Promise { + const block = await this.getBlockStore(blockNumber) + const values = await block.get(ZERO, MAX_BIG_NUM) + return values.map((value) => { + return JSON.parse(value.toString()) + }) + } +} diff --git a/packages/core/src/app/block-production/block-manager.ts b/packages/core/src/app/block-production/block-manager.ts new file mode 100644 index 00000000..284e1977 --- /dev/null +++ b/packages/core/src/app/block-production/block-manager.ts @@ -0,0 +1,59 @@ +import BigNum = require('bn.js') + +import { + BlockDB, + BlockManager, + CommitmentContract, +} from '../../types/block-production' +import { StateUpdate } from '../../types/serialization' + +/** + * Simple BlockManager implementation. + */ +export class DefaultBlockManager implements BlockManager { + /** + * Initializes the manager. + * @param blockdb BlockDB instance to store/query data from. + * @param commitmentContract Contract wrapper used to publish block roots. + */ + constructor( + private blockdb: BlockDB, + private commitmentContract: CommitmentContract + ) {} + + /** + * @returns the next plasma block number. + */ + public async getNextBlockNumber(): Promise { + return this.blockdb.getNextBlockNumber() + } + + /** + * Adds a state update to the list of updates to be published in the next + * plasma block. + * @param stateUpdate State update to add to the next block. + * @returns a promise that resolves once the update has been added. + */ + public async addPendingStateUpdate(stateUpdate: StateUpdate): Promise { + await this.blockdb.addPendingStateUpdate(stateUpdate) + } + + /** + * @returns the state updates to be published in the next block. + */ + public async getPendingStateUpdates(): Promise { + return this.blockdb.getPendingStateUpdates() + } + + /** + * Finalizes the next block and submits the block root to Ethereum. + * @returns a promise that resolves once the block has been published. + */ + public async submitNextBlock(): Promise { + // TODO: Lock / figure out concurrency + const blockNumber = await this.getNextBlockNumber() + await this.blockdb.finalizeNextBlock() + const root = await this.blockdb.getMerkleRoot(blockNumber) + await this.commitmentContract.submitBlock(root) + } +} diff --git a/packages/core/src/app/block-production/index.ts b/packages/core/src/app/block-production/index.ts index bef01883..3ad92070 100644 --- a/packages/core/src/app/block-production/index.ts +++ b/packages/core/src/app/block-production/index.ts @@ -1,3 +1,5 @@ +export * from './block-db' +export * from './block-manager' export * from './merkle-interval-tree' export * from './state-interval-tree' export * from './plasma-block-tree' diff --git a/packages/core/src/app/block-production/plasma-block-tree.ts b/packages/core/src/app/block-production/plasma-block-tree.ts index 271c96bd..cfcfd622 100644 --- a/packages/core/src/app/block-production/plasma-block-tree.ts +++ b/packages/core/src/app/block-production/plasma-block-tree.ts @@ -42,7 +42,7 @@ export class PlasmaBlock extends GenericMerkleIntervalTree /** * Returns a double inclusion proof which demonstrates the existence of a state update within the plasma block. - * @param stateUpdatePosition index of the state udpate in the state subtree of the block. + * @param stateUpdatePosition index of the state update in the state subtree of the block. * @param assetIdPosition index of the assetId in the top-level asset id of the block */ public getStateUpdateInclusionProof( diff --git a/packages/core/src/app/db/range-bucket.ts b/packages/core/src/app/db/range-bucket.ts index 28cb70c2..98220920 100644 --- a/packages/core/src/app/db/range-bucket.ts +++ b/packages/core/src/app/db/range-bucket.ts @@ -234,6 +234,11 @@ export class BaseRangeBucket implements RangeBucket { return ranges } + public async hasDataInRange(start: BigNum, end: BigNum): Promise { + // TODO: can eagerly return when true, but this is good enough or now + return (await this.get(start, end)).length > 0 + } + /** * Gets all ranges which intersect with [start,end) * @param start The start of the range we are getting. diff --git a/packages/core/src/app/ovm/state-manager.ts b/packages/core/src/app/ovm/state-manager.ts index e35e0dff..a68c9fce 100644 --- a/packages/core/src/app/ovm/state-manager.ts +++ b/packages/core/src/app/ovm/state-manager.ts @@ -24,13 +24,10 @@ import { getOverlappingRange, ONE, rangesIntersect } from '../../app' * See: http://spec.plasma.group/en/latest/src/05-client-architecture/state-manager.html for more details. */ export class DefaultStateManager implements StateManager { - private stateDB: StateDB - private pluginManager: PluginManager - - public constructor(stateDB: StateDB, pluginManager: PluginManager) { - this.stateDB = stateDB - this.pluginManager = pluginManager - } + public constructor( + private stateDB: StateDB, + private pluginManager: PluginManager + ) {} public async executeTransaction( transaction: Transaction, diff --git a/packages/core/src/app/utils/numbers.ts b/packages/core/src/app/utils/numbers.ts index 0e877419..de5eb919 100644 --- a/packages/core/src/app/utils/numbers.ts +++ b/packages/core/src/app/utils/numbers.ts @@ -2,3 +2,4 @@ import BigNum = require('bn.js') export const ZERO = new BigNum(0) export const ONE = new BigNum(1) +export const MAX_BIG_NUM = new BigNum('0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF') diff --git a/packages/core/src/types/aggregator/index.ts b/packages/core/src/types/aggregator/index.ts index 7938b439..07db4453 100644 --- a/packages/core/src/types/aggregator/index.ts +++ b/packages/core/src/types/aggregator/index.ts @@ -1,2 +1 @@ export * from './aggregator.interface' -export * from './block-manager.interface' diff --git a/packages/core/src/types/block-production/block-db.interface.ts b/packages/core/src/types/block-production/block-db.interface.ts new file mode 100644 index 00000000..6254f1c1 --- /dev/null +++ b/packages/core/src/types/block-production/block-db.interface.ts @@ -0,0 +1,11 @@ +import BigNum = require('bn.js') + +import { StateUpdate } from '../serialization' + +export interface BlockDB { + getNextBlockNumber(): Promise + addPendingStateUpdate(stateUpdate: StateUpdate): Promise + getPendingStateUpdates(): Promise + getMerkleRoot(blockNumber: BigNum): Promise + finalizeNextBlock(): Promise +} diff --git a/packages/core/src/types/aggregator/block-manager.interface.ts b/packages/core/src/types/block-production/block-manager.interface.ts similarity index 100% rename from packages/core/src/types/aggregator/block-manager.interface.ts rename to packages/core/src/types/block-production/block-manager.interface.ts diff --git a/packages/core/src/types/block-production/commitment-contract.interface.ts b/packages/core/src/types/block-production/commitment-contract.interface.ts new file mode 100644 index 00000000..6dc82f9c --- /dev/null +++ b/packages/core/src/types/block-production/commitment-contract.interface.ts @@ -0,0 +1,3 @@ +export interface CommitmentContract { + submitBlock(root: Buffer): Promise +} diff --git a/packages/core/src/types/block-production/index.ts b/packages/core/src/types/block-production/index.ts new file mode 100644 index 00000000..55e38ef8 --- /dev/null +++ b/packages/core/src/types/block-production/index.ts @@ -0,0 +1,3 @@ +export * from './block-manager.interface' +export * from './block-db.interface' +export * from './commitment-contract.interface' diff --git a/packages/core/src/types/db/range-db.interface.ts b/packages/core/src/types/db/range-db.interface.ts index 264bbeb2..81e54929 100644 --- a/packages/core/src/types/db/range-db.interface.ts +++ b/packages/core/src/types/db/range-db.interface.ts @@ -26,6 +26,14 @@ export interface RangeStore { readonly keyLength: number // The number of bytes which should be used for the range keys readonly endianness: Endianness // The endianness of the range keys + /** + * Determines if there is any data with a Range overlapping the specified Range. + * @param start the start of the Range + * @param end the end of the Range + * @returns true if there is data in the Range, false otherwise + */ + hasDataInRange(start: BigNum, end: BigNum): Promise + /** * Queries for all values which are stored over the particular range. * @param start the start of the range to query. diff --git a/packages/core/test/app/aggregator/aggregator.spec.ts b/packages/core/test/app/aggregator/aggregator.spec.ts index 857c3f54..01b96b77 100644 --- a/packages/core/test/app/aggregator/aggregator.spec.ts +++ b/packages/core/test/app/aggregator/aggregator.spec.ts @@ -3,7 +3,6 @@ import { should } from '../../setup' import { Aggregator, - BlockManager, BlockTransaction, BlockTransactionCommitment, HistoryProof, @@ -24,6 +23,7 @@ import { } from '../../../src/app/' import { TestUtils } from '../utils/test-utils' import * as assert from 'assert' +import { BlockManager } from '../../../src/types/block-production' /******************* * Mocks & Helpers * From 8a2682e8339cea2a14bad702f8f6fb7b5ebd379a Mon Sep 17 00:00:00 2001 From: Will Meister Date: Wed, 10 Jul 2019 15:56:24 -0500 Subject: [PATCH 04/15] Adding async-mutex lib and adding mutual exclusion around BlockDB blocks that are not safely concurrent --- packages/core/package.json | 1 + .../core/src/app/block-production/block-db.ts | 47 ++++++++++++------- yarn.lock | 5 ++ 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/packages/core/package.json b/packages/core/package.json index 52b05768..5352641e 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -29,6 +29,7 @@ }, "dependencies": { "abstract-leveldown": "^6.0.3", + "async-mutex": "^0.1.3", "axios": "^0.19.0", "bn.js": "^4.11.8", "body-parser": "^1.19.0", diff --git a/packages/core/src/app/block-production/block-db.ts b/packages/core/src/app/block-production/block-db.ts index e774bbc5..8d4440cc 100644 --- a/packages/core/src/app/block-production/block-db.ts +++ b/packages/core/src/app/block-production/block-db.ts @@ -1,5 +1,7 @@ /* External Imports */ import BigNum = require('bn.js') +import { Mutex, MutexInterface } from 'async-mutex' + import { BaseKey, BaseRangeBucket } from '../db' import { BlockDB } from '../../types/block-production' import { KeyValueStore, RangeStore } from '../../types/db' @@ -21,6 +23,7 @@ const KEYS = { * Simple BlockDB implementation. */ export class DefaultBlockDB implements BlockDB { + private readonly blockMutex: Mutex private vars: KeyValueStore private blocks: KeyValueStore @@ -31,6 +34,7 @@ export class DefaultBlockDB implements BlockDB { constructor(private db: KeyValueStore) { this.vars = this.db.bucket(PREFIXES.VARS) this.blocks = this.db.bucket(PREFIXES.BLOCKS) + this.blockMutex = new Mutex() } /** @@ -55,19 +59,20 @@ export class DefaultBlockDB implements BlockDB { * @returns a promise that resolves once the update has been added. */ public async addPendingStateUpdate(stateUpdate: StateUpdate): Promise { - const block = await this.getNextBlockStore() - const start = stateUpdate.range.start - const end = stateUpdate.range.end - - // TODO: Figure out how to implement locking here so two state updates - // can't be added at the same time. - - if (await block.hasDataInRange(start, end)) { - throw new Error('Block already contains a state update over that range.') - } + await this.blockMutex.runExclusive(async () => { + const block = await this.getNextBlockStore() + const start = stateUpdate.range.start + const end = stateUpdate.range.end + + if (await block.hasDataInRange(start, end)) { + throw new Error( + 'Block already contains a state update over that range.' + ) + } - const value = Buffer.from(JSON.stringify(stateUpdate)) - await block.put(start, end, value) + const value = Buffer.from(JSON.stringify(stateUpdate)) + await block.put(start, end, value) + }) } /** @@ -104,11 +109,13 @@ export class DefaultBlockDB implements BlockDB { * Finalizes the next plasma block so that it can be published. */ public async finalizeNextBlock(): Promise { - // TODO: manage concurrency here - const prevBlockNumber = await this.getNextBlockNumber() - const nextBlockNumber = Buffer.allocUnsafe(4) - nextBlockNumber.writeUInt32BE(prevBlockNumber.add(ONE).toNumber(), 0) - await this.vars.put(KEYS.NEXT_BLOCK, nextBlockNumber) + await this.blockMutex.runExclusive(async () => { + const prevBlockNumber = await this.getNextBlockNumber() + const nextBlockNumber = Buffer.allocUnsafe(4) + + nextBlockNumber.writeUInt32BE(prevBlockNumber.add(ONE).toNumber(), 0) + await this.vars.put(KEYS.NEXT_BLOCK, nextBlockNumber) + }) } /** @@ -124,6 +131,12 @@ export class DefaultBlockDB implements BlockDB { /** * @returns the RangeDB instance for the next block to be published. + * + * IMPORTANT: This function itself is safe from concurrency issues, but + * if the caller is modifying the returned RangeStore or needs to + * guarantee the returned next RangeStore is not stale, both the call + * to this function AND any subsequent reads / writes should be run with + * the blockMutex lock held to guarantee the expected behavior. */ private async getNextBlockStore(): Promise { const blockNumber = await this.getNextBlockNumber() diff --git a/yarn.lock b/yarn.lock index 45c15849..07a59421 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1247,6 +1247,11 @@ async-limiter@~1.0.0: resolved "https://registry.yarnpkg.com/async-limiter/-/async-limiter-1.0.0.tgz#78faed8c3d074ab81f22b4e985d79e8738f720f8" integrity sha512-jp/uFnooOiO+L211eZOoSyzpOITMXx1rBITauYykG3BRYPu8h0UcxsPNB04RR5vo4Tyz3+ay17tR6JVf9qzYWg== +async-mutex@^0.1.3: + version "0.1.3" + resolved "https://registry.yarnpkg.com/async-mutex/-/async-mutex-0.1.3.tgz#0aad2112369795ab3f17e33744556d2ecf547566" + integrity sha1-Cq0hEjaXlas/F+M3RFVtLs9UdWY= + async@2.6.1: version "2.6.1" resolved "https://registry.yarnpkg.com/async/-/async-2.6.1.tgz#b245a23ca71930044ec53fa46aa00a3e87c6a610" From afaf7461b496a525ec6d0addc73ccb9a851ab12a Mon Sep 17 00:00:00 2001 From: Will Meister Date: Thu, 11 Jul 2019 14:57:36 -0500 Subject: [PATCH 05/15] Adding BlockManager tests and handling mutual exclusion in Block Submission --- .../core/src/app/block-production/block-db.ts | 2 +- .../src/app/block-production/block-manager.ts | 22 +- .../block-production/block-manager.spec.ts | 202 ++++++++++++++++++ 3 files changed, 219 insertions(+), 7 deletions(-) create mode 100644 packages/core/test/app/block-production/block-manager.spec.ts diff --git a/packages/core/src/app/block-production/block-db.ts b/packages/core/src/app/block-production/block-db.ts index 8d4440cc..f5e52fa4 100644 --- a/packages/core/src/app/block-production/block-db.ts +++ b/packages/core/src/app/block-production/block-db.ts @@ -1,6 +1,6 @@ /* External Imports */ import BigNum = require('bn.js') -import { Mutex, MutexInterface } from 'async-mutex' +import { Mutex } from 'async-mutex' import { BaseKey, BaseRangeBucket } from '../db' import { BlockDB } from '../../types/block-production' diff --git a/packages/core/src/app/block-production/block-manager.ts b/packages/core/src/app/block-production/block-manager.ts index 284e1977..4d7f1589 100644 --- a/packages/core/src/app/block-production/block-manager.ts +++ b/packages/core/src/app/block-production/block-manager.ts @@ -1,4 +1,5 @@ import BigNum = require('bn.js') +import { Mutex } from 'async-mutex' import { BlockDB, @@ -11,6 +12,8 @@ import { StateUpdate } from '../../types/serialization' * Simple BlockManager implementation. */ export class DefaultBlockManager implements BlockManager { + private readonly blockSubmissionMutex: Mutex + /** * Initializes the manager. * @param blockdb BlockDB instance to store/query data from. @@ -19,7 +22,9 @@ export class DefaultBlockManager implements BlockManager { constructor( private blockdb: BlockDB, private commitmentContract: CommitmentContract - ) {} + ) { + this.blockSubmissionMutex = new Mutex() + } /** * @returns the next plasma block number. @@ -50,10 +55,15 @@ export class DefaultBlockManager implements BlockManager { * @returns a promise that resolves once the block has been published. */ public async submitNextBlock(): Promise { - // TODO: Lock / figure out concurrency - const blockNumber = await this.getNextBlockNumber() - await this.blockdb.finalizeNextBlock() - const root = await this.blockdb.getMerkleRoot(blockNumber) - await this.commitmentContract.submitBlock(root) + await this.blockSubmissionMutex.runExclusive(async () => { + // Don't submit the block if there are no StateUpdates + if ((await this.getPendingStateUpdates()).length === 0) { + return + } + const blockNumber = await this.getNextBlockNumber() + await this.blockdb.finalizeNextBlock() + const root = await this.blockdb.getMerkleRoot(blockNumber) + await this.commitmentContract.submitBlock(root) + }) } } diff --git a/packages/core/test/app/block-production/block-manager.spec.ts b/packages/core/test/app/block-production/block-manager.spec.ts new file mode 100644 index 00000000..1314e509 --- /dev/null +++ b/packages/core/test/app/block-production/block-manager.spec.ts @@ -0,0 +1,202 @@ +import BigNum = require('bn.js') +import * as assert from 'assert' +import { should } from '../../setup' + +import { StateUpdate } from '../../../src/types' +import { DefaultBlockManager, ONE, stateUpdatesEqual } from '../../../src/app/' +import { TestUtils } from '../utils/test-utils' +import { + BlockDB, + BlockManager, + CommitmentContract, +} from '../../../src/types/block-production' + +/******************* + * Mocks & Helpers * + *******************/ + +class DummyCommitmentContract implements CommitmentContract { + private throwOnSubmit: boolean = false + + public setThrowOnSubmit() { + this.throwOnSubmit = true + } + + public async submitBlock(root: Buffer): Promise { + if (this.throwOnSubmit) { + this.throwOnSubmit = false + throw Error('Simulating error submitting block') + } + } +} + +class DummyBlockDB implements BlockDB { + private nextBlockNumber: BigNum = ONE + private pendingStateUpdates: StateUpdate[] = [] + private throwOnFinalize: boolean = false + + public async addPendingStateUpdate(stateUpdate: StateUpdate): Promise { + this.pendingStateUpdates.push(stateUpdate) + } + + public setThrowOnFinalize(): void { + this.throwOnFinalize = true + } + + public async finalizeNextBlock(): Promise { + if (this.throwOnFinalize) { + this.throwOnFinalize = false + throw Error('Simulating error finalizing block') + } + this.pendingStateUpdates.length = 0 + this.nextBlockNumber = this.nextBlockNumber.add(ONE) + } + + public async getMerkleRoot(blockNumber: BigNum): Promise { + return Buffer.from('placeholder') + } + + public async getNextBlockNumber(): Promise { + return this.nextBlockNumber + } + + public async getPendingStateUpdates(): Promise { + return this.pendingStateUpdates + } +} + +const addStateUpdateToBlockManager = async ( + blockManager: BlockManager +): Promise => { + const stateUpdate: StateUpdate = TestUtils.generateNSequentialStateUpdates( + 1 + )[0] + await blockManager.addPendingStateUpdate(stateUpdate) + + const returnedUpdates: StateUpdate[] = await blockManager.getPendingStateUpdates() + assert( + !!returnedUpdates && returnedUpdates.length === 1, + `getPendingStateUpdates returned undefined or empty list when expecting a single StateUpdate. returned: ${JSON.stringify( + returnedUpdates + )}` + ) +} + +/********* + * TESTS * + *********/ + +describe('DefaultBlockManager', () => { + let blockManager: BlockManager + let blockDB: DummyBlockDB + let commitmentContract: DummyCommitmentContract + + beforeEach(async () => { + blockDB = new DummyBlockDB() + commitmentContract = new DummyCommitmentContract() + blockManager = new DefaultBlockManager(blockDB, commitmentContract) + }) + + describe('getNextBlockNumber', () => { + it('is in sync with BlockDB', async () => { + const blockDBNextBlock: BigNum = await blockDB.getNextBlockNumber() + const blockManagerNextBlock: BigNum = await blockManager.getNextBlockNumber() + + assert( + blockManagerNextBlock.eq(blockDBNextBlock), + 'BlockDB and BlockManager are out of sync' + ) + }) + }) + + describe('addPendingStateUpdate / getPendingStateUpdates', () => { + it('stores the pending StateUpdate(s)', async () => { + const stateUpdates: StateUpdate[] = TestUtils.generateNSequentialStateUpdates( + 2 + ) + await blockManager.addPendingStateUpdate(stateUpdates[0]) + + let returnedUpdates: StateUpdate[] = await blockManager.getPendingStateUpdates() + + assert( + !!returnedUpdates && returnedUpdates.length === 1, + `getPendingStateUpdates returned undefined or empty list when expecting a single StateUpdate. returned: ${JSON.stringify( + returnedUpdates + )}` + ) + assert( + stateUpdatesEqual(returnedUpdates[0], stateUpdates[0]), + 'Added StateUpdate is not the same as returned StateUpdate.' + ) + + await blockManager.addPendingStateUpdate(stateUpdates[1]) + returnedUpdates = await blockManager.getPendingStateUpdates() + + assert( + !!returnedUpdates && returnedUpdates.length === 2, + `getPendingStateUpdates returned undefined or empty list when expecting a 2 StateUpdates. returned: ${JSON.stringify( + returnedUpdates + )}` + ) + assert( + (stateUpdatesEqual(returnedUpdates[0], stateUpdates[0]) || + stateUpdatesEqual(returnedUpdates[1], stateUpdates[0])) && + (stateUpdatesEqual(returnedUpdates[0], stateUpdates[1]) || + stateUpdatesEqual(returnedUpdates[1], stateUpdates[1])), + 'Added StateUpdates are not the same as returned StateUpdates.' + ) + }) + }) + + describe('submitNextBlock', () => { + it('increments next block and clears pending StateUpdates', async () => { + const previousNextBlockNumber: BigNum = await blockManager.getNextBlockNumber() + + await addStateUpdateToBlockManager(blockManager) + + await blockManager.submitNextBlock() + + const updatedNextBlockNumber: BigNum = await blockManager.getNextBlockNumber() + assert( + updatedNextBlockNumber.eq(previousNextBlockNumber.add(ONE)), + `Block Number after submitted block is not incremented. Got ${updatedNextBlockNumber.toString()}, expected ${previousNextBlockNumber + .add(ONE) + .toString()}` + ) + + const returnedUpdates = await blockManager.getPendingStateUpdates() + assert( + !!returnedUpdates && returnedUpdates.length === 0, + `getPendingStateUpdates returned undefined or non-empty list when expecting an empty list. returned: ${JSON.stringify( + returnedUpdates + )}` + ) + }) + + it('throws if block submission fails', async () => { + await addStateUpdateToBlockManager(blockManager) + + commitmentContract.setThrowOnSubmit() + + try { + await blockManager.submitNextBlock() + assert(false, 'This should have thrown.') + } catch (e) { + // This is success + } + }) + + it('throws if block finalization fails', async () => { + await addStateUpdateToBlockManager(blockManager) + + blockDB.setThrowOnFinalize() + + try { + await blockManager.submitNextBlock() + assert(false, 'This should have thrown.') + } catch (e) { + // This is success + } + }) + }) +}) From 9b60b71c0b4ce1e9051e23ab1a2efe693c44384f Mon Sep 17 00:00:00 2001 From: Will Meister Date: Thu, 11 Jul 2019 15:02:48 -0500 Subject: [PATCH 06/15] Adding test to make sure empty blocks are not submitted --- .../test/app/block-production/block-manager.spec.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/core/test/app/block-production/block-manager.spec.ts b/packages/core/test/app/block-production/block-manager.spec.ts index 1314e509..afc8dd08 100644 --- a/packages/core/test/app/block-production/block-manager.spec.ts +++ b/packages/core/test/app/block-production/block-manager.spec.ts @@ -173,6 +173,17 @@ describe('DefaultBlockManager', () => { ) }) + it('does not submit block if there are no updates', async () => { + const previousNextBlockNumber: BigNum = await blockManager.getNextBlockNumber() + await blockManager.submitNextBlock() + const updatedNextBlockNumber: BigNum = await blockManager.getNextBlockNumber() + + assert( + previousNextBlockNumber.eq(updatedNextBlockNumber), + 'Block was incremented when submission should not have taken place.' + ) + }) + it('throws if block submission fails', async () => { await addStateUpdateToBlockManager(blockManager) From 3d91ac86f118adf9b8dfd9dd89bd2327874dee7c Mon Sep 17 00:00:00 2001 From: Will Meister Date: Mon, 15 Jul 2019 15:20:28 -0500 Subject: [PATCH 07/15] - Adding tests for BlockDB - Changing BlockDB to have buffer keys for BigNums - Adding serialization and deserialization for StateUpdate to serialize/parse objects to/from string --- .../core/src/app/block-production/block-db.ts | 47 +++--- packages/core/src/app/db/db.ts | 14 +- packages/core/src/app/db/range-bucket.ts | 7 +- .../src/app/serialization/state-object.ts | 2 +- .../src/app/serialization/state-update.ts | 29 ++++ packages/core/src/types/db/db.interface.ts | 3 + .../app/block-production/block-db.spec.ts | 135 ++++++++++++++++++ packages/core/tslint.json | 3 +- 8 files changed, 206 insertions(+), 34 deletions(-) create mode 100644 packages/core/test/app/block-production/block-db.spec.ts diff --git a/packages/core/src/app/block-production/block-db.ts b/packages/core/src/app/block-production/block-db.ts index f5e52fa4..d16ad441 100644 --- a/packages/core/src/app/block-production/block-db.ts +++ b/packages/core/src/app/block-production/block-db.ts @@ -8,15 +8,11 @@ import { KeyValueStore, RangeStore } from '../../types/db' import { StateUpdate } from '../../types/serialization' import { MAX_BIG_NUM, ONE, ZERO } from '../utils' import { GenericMerkleIntervalTree } from './merkle-interval-tree' - -const PREFIXES = { - VARS: new BaseKey('v').encode(), - BLOCKS: new BaseKey('b').encode(), -} +import { deserializeStateUpdate, serializeStateUpdate } from '../serialization' const KEYS = { NEXT_BLOCK: Buffer.from('nextblock'), - BLOCK: new BaseKey('b', ['uint32']), + BLOCK: new BaseKey('b', ['buffer']), } /** @@ -24,16 +20,16 @@ const KEYS = { */ export class DefaultBlockDB implements BlockDB { private readonly blockMutex: Mutex - private vars: KeyValueStore - private blocks: KeyValueStore /** * Initializes the database wrapper. - * @param db Database to store values in. + * @param vars the KeyValueStore to store variables in + * @param blocks the KeyValueStore to store Blocks in */ - constructor(private db: KeyValueStore) { - this.vars = this.db.bucket(PREFIXES.VARS) - this.blocks = this.db.bucket(PREFIXES.BLOCKS) + constructor( + private readonly vars: KeyValueStore, + private readonly blocks: KeyValueStore + ) { this.blockMutex = new Mutex() } @@ -41,15 +37,9 @@ export class DefaultBlockDB implements BlockDB { * @returns the next plasma block number. */ public async getNextBlockNumber(): Promise { - // NOTE: You could theoretically cache the next block number as to avoid - // having to read from the database repeatedly. However, this introduces - // the need to ensure that the cached value and stored value never fall out - // of sync. It's probably better to hold off on implementing caching until - // this becomes a performance bottleneck. - + // TODO: Cache this when it makes sense const buf = await this.vars.get(KEYS.NEXT_BLOCK) - // TODO: Node 12 has Buffer.readBigInt64BE(...) -- upgrade? - return new BigNum(buf.readUInt32BE(0)) + return !buf ? ONE : new BigNum(buf, 'be') } /** @@ -70,7 +60,7 @@ export class DefaultBlockDB implements BlockDB { ) } - const value = Buffer.from(JSON.stringify(stateUpdate)) + const value = Buffer.from(serializeStateUpdate(stateUpdate)) await block.put(start, end, value) }) } @@ -94,7 +84,7 @@ export class DefaultBlockDB implements BlockDB { const leaves = stateUpdates.map((stateUpdate) => { // TODO: Actually encode this. - const encodedStateUpdate = JSON.stringify(stateUpdate) + const encodedStateUpdate = serializeStateUpdate(stateUpdate) return { start: stateUpdate.range.start, end: stateUpdate.range.end, @@ -107,13 +97,16 @@ export class DefaultBlockDB implements BlockDB { /** * Finalizes the next plasma block so that it can be published. + * + * Note: The execution of this function is serialized internally, + * but to be of use, the caller will most likely want to serialize + * their calls to it as well. */ public async finalizeNextBlock(): Promise { await this.blockMutex.runExclusive(async () => { - const prevBlockNumber = await this.getNextBlockNumber() - const nextBlockNumber = Buffer.allocUnsafe(4) + const prevBlockNumber: BigNum = await this.getNextBlockNumber() + const nextBlockNumber: Buffer = prevBlockNumber.add(ONE).toBuffer('be') - nextBlockNumber.writeUInt32BE(prevBlockNumber.add(ONE).toNumber(), 0) await this.vars.put(KEYS.NEXT_BLOCK, nextBlockNumber) }) } @@ -124,7 +117,7 @@ export class DefaultBlockDB implements BlockDB { * @returns the RangeDB instance for the given block. */ private async getBlockStore(blockNumber: BigNum): Promise { - const key = KEYS.BLOCK.encode([blockNumber]) + const key = KEYS.BLOCK.encode([blockNumber.toBuffer('be')]) const bucket = this.blocks.bucket(key) return new BaseRangeBucket(bucket.db, bucket.prefix) } @@ -152,7 +145,7 @@ export class DefaultBlockDB implements BlockDB { const block = await this.getBlockStore(blockNumber) const values = await block.get(ZERO, MAX_BIG_NUM) return values.map((value) => { - return JSON.parse(value.toString()) + return deserializeStateUpdate(value.value.toString()) }) } } diff --git a/packages/core/src/app/db/db.ts b/packages/core/src/app/db/db.ts index 48aeaf54..e5fdd3bf 100644 --- a/packages/core/src/app/db/db.ts +++ b/packages/core/src/app/db/db.ts @@ -4,7 +4,11 @@ */ /* External Imports */ -import { AbstractOpenOptions, AbstractLevelDOWN } from 'abstract-leveldown' +import { + AbstractOpenOptions, + AbstractLevelDOWN, + AbstractChainedBatch, +} from 'abstract-leveldown' /* Internal Imports */ import { @@ -16,12 +20,18 @@ import { Iterator, Bucket, RangeBucket, + KeyValueStore, + PutBatch, + PUT_BATCH_TYPE, + DEL_BATCH_TYPE, } from '../../types' import { BaseIterator } from './iterator' import { BaseBucket } from './bucket' import { BaseRangeBucket } from './range-bucket' import { bufferUtils } from '../../app' +export const DEFAULT_PREFIX_LENGTH = 3 + /** * Checks if an error is a NotFoundError. * @param err Error to check. @@ -45,7 +55,7 @@ const isNotFound = (err: any): boolean => { export class BaseDB implements DB { constructor( readonly db: AbstractLevelDOWN, - readonly prefixLength: number = 3 + readonly prefixLength: number = DEFAULT_PREFIX_LENGTH ) {} /** diff --git a/packages/core/src/app/db/range-bucket.ts b/packages/core/src/app/db/range-bucket.ts index 98220920..47b249bd 100644 --- a/packages/core/src/app/db/range-bucket.ts +++ b/packages/core/src/app/db/range-bucket.ts @@ -18,6 +18,7 @@ import { RangeBucket, RangeEntry, Endianness, + PUT_BATCH_TYPE, } from '../../types' import { bufferUtils, intersects, BaseDB } from '../../app' @@ -194,7 +195,7 @@ export class BaseRangeBucket implements RangeBucket { if (ranges.length > 0 && start.gt(ranges[0].start)) { // Reduce the first affected range's end position. Eg: ##### becomes ###$$ batchOps.push({ - type: 'put', + type: PUT_BATCH_TYPE, key: this.bnToKey(start), value: this.addStartToValue(ranges[0].start, ranges[0].value), }) @@ -202,7 +203,7 @@ export class BaseRangeBucket implements RangeBucket { // If the end position less than the last range's end... if (ranges.length > 0 && ranges[ranges.length - 1].end.gt(end)) { batchOps.push({ - type: 'put', + type: PUT_BATCH_TYPE, key: this.bnToKey(ranges[ranges.length - 1].end), value: this.addStartToValue(end, ranges[ranges.length - 1].value), }) @@ -211,7 +212,7 @@ export class BaseRangeBucket implements RangeBucket { // Step #3: Add our new range // batchOps.push({ - type: 'put', + type: PUT_BATCH_TYPE, key: this.bnToKey(end), value: this.addStartToValue(start, value), }) diff --git a/packages/core/src/app/serialization/state-object.ts b/packages/core/src/app/serialization/state-object.ts index def7a87d..9de8e399 100644 --- a/packages/core/src/app/serialization/state-object.ts +++ b/packages/core/src/app/serialization/state-object.ts @@ -1,6 +1,6 @@ /* Internal Imports */ import { abi } from '../../app' -import { StateObject, AbiEncodable } from '../../types' +import { StateObject, AbiEncodable, StateUpdate } from '../../types' /** * Creates a StateObject from an encoded StateObject. diff --git a/packages/core/src/app/serialization/state-update.ts b/packages/core/src/app/serialization/state-update.ts index 67b4e5f1..20e6e44a 100644 --- a/packages/core/src/app/serialization/state-update.ts +++ b/packages/core/src/app/serialization/state-update.ts @@ -26,6 +26,35 @@ const fromEncoded = (encoded: string): AbiStateUpdate => { ) } +/** + * Serializes a StateUpdate to a string. + * + * @param stateUpdate the StateUpdate to serialize + * @returns the serialized StateUpdate + */ +export const serializeStateUpdate = (stateUpdate: StateUpdate): string => { + return JSON.stringify(stateUpdate) +} + +/** + * Deserializes a StateUpdate, parsing it from a string. + * + * @param stateUpdate the string StateUpdate + * @returns the parsed StateUpdate + */ +export const deserializeStateUpdate = (stateUpdate: string): StateUpdate => { + const obj: {} = JSON.parse(stateUpdate) + return { + range: { + start: new BigNum(obj['range']['start'], 'hex'), + end: new BigNum(obj['range']['end'], 'hex'), + }, + stateObject: obj['stateObject'], + depositAddress: obj['depositAddress'], + plasmaBlockNumber: new BigNum(obj['plasmaBlockNumber']), + } +} + /** * Represents a basic abi encodable AbiStateUpdate */ diff --git a/packages/core/src/types/db/db.interface.ts b/packages/core/src/types/db/db.interface.ts index 77e74134..8256cb5e 100644 --- a/packages/core/src/types/db/db.interface.ts +++ b/packages/core/src/types/db/db.interface.ts @@ -15,6 +15,9 @@ export interface KV { value: V } +export const PUT_BATCH_TYPE = 'put' +export const DEL_BATCH_TYPE = 'del' + export type Batch = PutBatch | DelBatch export interface PutBatch { diff --git a/packages/core/test/app/block-production/block-db.spec.ts b/packages/core/test/app/block-production/block-db.spec.ts new file mode 100644 index 00000000..132ef522 --- /dev/null +++ b/packages/core/test/app/block-production/block-db.spec.ts @@ -0,0 +1,135 @@ +import BigNum = require('bn.js') +import MemDown from 'memdown' +import * as assert from 'assert' + +import { should } from '../../setup' +import { KeyValueStore } from '../../../src/types/db' +import { BaseBucket, BaseDB, DEFAULT_PREFIX_LENGTH } from '../../../src/app/db' +import { BlockDB } from '../../../src/types/block-production' +import { DefaultBlockDB } from '../../../src/app/block-production' +import { ONE, stateUpdatesEqual } from '../../../src/app/utils' +import { StateUpdate } from '../../../src/types/serialization' +import { TestUtils } from '../utils/test-utils' + +/******************* + * Mocks & Helpers * + *******************/ + +/********* + * TESTS * + *********/ + +describe('DefaultBlockDB', () => { + let varStore: KeyValueStore + let blockStore: KeyValueStore + let blockDB: BlockDB + + beforeEach(async () => { + varStore = new BaseBucket( + new BaseDB(new MemDown('') as any, DEFAULT_PREFIX_LENGTH * 2), + Buffer.from('v') + ) + blockStore = new BaseBucket( + new BaseDB(new MemDown('') as any, DEFAULT_PREFIX_LENGTH * 2), + Buffer.from('b') + ) + blockDB = new DefaultBlockDB(varStore, blockStore) + }) + + describe('getNextBlockNumber', () => { + it('returns 1 by default', async () => { + const nextBlock: BigNum = await blockDB.getNextBlockNumber() + assert( + nextBlock.eq(ONE), + `Next Block Number did not default to 1. Expected ${ONE.toString()}, got ${nextBlock.toString()}` + ) + }) + + it('increases when finalized', async () => { + await blockDB.finalizeNextBlock() + const nextBlock: BigNum = await blockDB.getNextBlockNumber() + const expected: BigNum = new BigNum(2) + assert( + nextBlock.eq(expected), + `Next Block didn't increase after block finalization. Expected ${expected.toString()}, got ${nextBlock.toString()}` + ) + }) + }) + + describe('addPendingStateUpdate / getPendingStateUpdates', () => { + it('stores the pending StateUpdate(s)', async () => { + const stateUpdates: StateUpdate[] = TestUtils.generateNSequentialStateUpdates( + 2 + ) + await blockDB.addPendingStateUpdate(stateUpdates[0]) + + let returnedUpdates: StateUpdate[] = await blockDB.getPendingStateUpdates() + + assert( + !!returnedUpdates && returnedUpdates.length === 1, + `getPendingStateUpdates returned undefined or empty list when expecting a single StateUpdate. returned: ${JSON.stringify( + returnedUpdates + )}` + ) + + assert( + stateUpdatesEqual(returnedUpdates[0], stateUpdates[0]), + 'Added StateUpdate is not the same as returned StateUpdate.' + ) + + await blockDB.addPendingStateUpdate(stateUpdates[1]) + returnedUpdates = await blockDB.getPendingStateUpdates() + + assert( + !!returnedUpdates && returnedUpdates.length === 2, + `getPendingStateUpdates returned undefined or empty list when expecting a 2 StateUpdates. returned: ${JSON.stringify( + returnedUpdates + )}` + ) + assert( + (stateUpdatesEqual(returnedUpdates[0], stateUpdates[0]) || + stateUpdatesEqual(returnedUpdates[1], stateUpdates[0])) && + (stateUpdatesEqual(returnedUpdates[0], stateUpdates[1]) || + stateUpdatesEqual(returnedUpdates[1], stateUpdates[1])), + 'Added StateUpdates are not the same as returned StateUpdates.' + ) + }) + }) + + describe('finalizeNextBlock', () => { + it('increments next block and clears pending StateUpdates', async () => { + const previousNextBlockNumber: BigNum = await blockDB.getNextBlockNumber() + + const stateUpdate: StateUpdate = TestUtils.generateNSequentialStateUpdates( + 1 + )[0] + await blockDB.addPendingStateUpdate(stateUpdate) + + let returnedUpdates: StateUpdate[] = await blockDB.getPendingStateUpdates() + assert( + !!returnedUpdates && returnedUpdates.length === 1, + `getPendingStateUpdates returned undefined or empty list when expecting a single StateUpdate. returned: ${JSON.stringify( + returnedUpdates + )}` + ) + + await blockDB.finalizeNextBlock() + + const updatedNextBlockNumber: BigNum = await blockDB.getNextBlockNumber() + assert( + updatedNextBlockNumber.eq(previousNextBlockNumber.add(ONE)), + `Block Number after submitted block is not incremented. Got ${updatedNextBlockNumber.toString()}, expected ${previousNextBlockNumber + .add(ONE) + .toString()}` + ) + + returnedUpdates = await blockDB.getPendingStateUpdates() + assert( + !!returnedUpdates && returnedUpdates.length === 0, + `getPendingStateUpdates returned undefined or non-empty list when expecting an empty list. returned: ${JSON.stringify( + returnedUpdates + )}` + ) + }) + }) +}) diff --git a/packages/core/tslint.json b/packages/core/tslint.json index e5e65be8..85e34e9c 100644 --- a/packages/core/tslint.json +++ b/packages/core/tslint.json @@ -1,6 +1,7 @@ { "extends": ["./../../tslint.json"], "rules": { - "prettier": [true, "./.prettierrc.js"] + "prettier": [true, "./.prettierrc.js"], + "no-string-literal": false // Allows obj['property'] access } } From 0dc31ef341ef0ffd8579334caa9a90a0defda38c Mon Sep 17 00:00:00 2001 From: Will Meister Date: Mon, 15 Jul 2019 15:35:46 -0500 Subject: [PATCH 08/15] Changing tslint rule to be repo-wide --- packages/core/tslint.json | 3 +-- tslint.json | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/core/tslint.json b/packages/core/tslint.json index 85e34e9c..e5e65be8 100644 --- a/packages/core/tslint.json +++ b/packages/core/tslint.json @@ -1,7 +1,6 @@ { "extends": ["./../../tslint.json"], "rules": { - "prettier": [true, "./.prettierrc.js"], - "no-string-literal": false // Allows obj['property'] access + "prettier": [true, "./.prettierrc.js"] } } diff --git a/tslint.json b/tslint.json index b3ab94ea..af3cb18f 100644 --- a/tslint.json +++ b/tslint.json @@ -5,18 +5,19 @@ "tslint-plugin-prettier" ], "rules": { - "prettier": true, - "no-unused-expression": false, + "ban-types": false, "interface-name": false, - "object-literal-sort-keys": false, - "ordered-imports": false, - "no-submodule-imports": false, - "variable-name": false, - "no-empty-interface": false, "max-classes-per-file": false, + "member-ordering": false, + "no-empty-interface": false, "no-implicit-dependencies": [true, "dev"], - "ban-types": false, - "member-ordering": false + "no-string-literal": false, + "no-submodule-imports": false, + "no-unused-expression": false, + "object-literal-sort-keys": false, + "ordered-imports": false, + "prettier": true, + "variable-name": false }, "linterOptions": { "exclude": [ From 1c8f514ac4750d98f510d6aefcd9989d6a87aad5 Mon Sep 17 00:00:00 2001 From: Will Meister Date: Mon, 15 Jul 2019 17:02:52 -0500 Subject: [PATCH 09/15] Adding examples and comments in rangesSpanRange for clarity --- packages/core/src/app/utils/range.ts | 21 ++++++++++++++------- tslint.json | 2 +- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/core/src/app/utils/range.ts b/packages/core/src/app/utils/range.ts index 8d530023..5daec36c 100644 --- a/packages/core/src/app/utils/range.ts +++ b/packages/core/src/app/utils/range.ts @@ -66,7 +66,11 @@ export const isRangeSubset = (subset: Range, superset: Range): boolean => { } /** - * Determines whether the provided Ranges collectively span the Range in question + * Determines whether the provided Ranges collectively span the Range in question. + * For instance, + * doRangesSpanRange([{start: 1, end: 3}, {start: 3, end: 5}], {start: 2, end: 4}) + * returns true because there is no number in range that is not covered by at + * least one element in ranges. * * @param ranges the Ranges that will/won't span the rangeToSpan * @param rangeToSpan the Range being spanned @@ -76,21 +80,24 @@ export const rangesSpanRange = ( ranges: Range[], rangeToSpan: Range ): boolean => { + // Sorting the ranges by Start so we can go through them sequentially const sortedRanges: Range[] = ranges.sort((a: Range, b: Range) => { return a.start.lt(b.start) ? -1 : a.start.eq(b.start) ? 0 : 1 }) - let spannedEnd: BigNum = rangeToSpan.start + let lowestNotSpanned: BigNum = rangeToSpan.start for (const rangeElem of sortedRanges) { - // If our lowest range start is greater than our spannedEnd, the range cannot be spanned - if (rangeElem.start.gt(spannedEnd)) { + // If our lowest range start is greater than our lowestNotSpanned, + // the range cannot be spanned because the Ranges do not include lowestNotSpanned. + if (rangeElem.start.gt(lowestNotSpanned)) { return false } - spannedEnd = rangeElem.end + // Now we've covered rangeElem.start - rangeElem.end, so update lowestNotSpanned + lowestNotSpanned = rangeElem.end - // If the entire range has been spanned we can , - if (spannedEnd.gte(rangeToSpan.end)) { + // If the entire range has been spanned we can return true + if (lowestNotSpanned.gte(rangeToSpan.end)) { return true } } diff --git a/tslint.json b/tslint.json index af3cb18f..4c47a66d 100644 --- a/tslint.json +++ b/tslint.json @@ -11,7 +11,7 @@ "member-ordering": false, "no-empty-interface": false, "no-implicit-dependencies": [true, "dev"], - "no-string-literal": false, + "no-string-literal": false, "no-submodule-imports": false, "no-unused-expression": false, "object-literal-sort-keys": false, From 2c7c789a6c2995804f5a391de72b2c3838add5b0 Mon Sep 17 00:00:00 2001 From: Will Meister Date: Mon, 15 Jul 2019 17:12:13 -0500 Subject: [PATCH 10/15] Adding error details --- packages/core/src/app/aggregator/aggregator.ts | 4 +++- packages/core/src/app/utils/range.ts | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/core/src/app/aggregator/aggregator.ts b/packages/core/src/app/aggregator/aggregator.ts index d100804d..0bceea64 100644 --- a/packages/core/src/app/aggregator/aggregator.ts +++ b/packages/core/src/app/aggregator/aggregator.ts @@ -39,7 +39,9 @@ export class DefaultAggregator implements Aggregator { if (!rangesSpanRange(validRanges, transaction.range)) { throw Error( - 'Cannot ingest Transaction that is not valid across its entire range.' + `Cannot ingest Transaction that is not valid across its entire range. + Valid Ranges: ${JSON.stringify(validRanges)}. + Transaction: ${JSON.stringify(transaction)}.` ) } diff --git a/packages/core/src/app/utils/range.ts b/packages/core/src/app/utils/range.ts index 5daec36c..6445965a 100644 --- a/packages/core/src/app/utils/range.ts +++ b/packages/core/src/app/utils/range.ts @@ -72,8 +72,8 @@ export const isRangeSubset = (subset: Range, superset: Range): boolean => { * returns true because there is no number in range that is not covered by at * least one element in ranges. * - * @param ranges the Ranges that will/won't span the rangeToSpan - * @param rangeToSpan the Range being spanned + * @param ranges the Ranges that, when combined will/won't span the rangeToSpan + * @param rangeToSpan the Range being evaluated * @returns true if ranges span rangeToSpan, false otherwise */ export const rangesSpanRange = ( From 7ce50990ca0ef6df48055e6c6d9355280052f024 Mon Sep 17 00:00:00 2001 From: Will Meister Date: Mon, 22 Jul 2019 13:27:21 -0500 Subject: [PATCH 11/15] renaming rangesSpanRange to doRangesSpanRange, removing unused import --- packages/core/src/app/aggregator/aggregator.ts | 4 ++-- packages/core/src/app/utils/equals.ts | 1 - packages/core/src/app/utils/range.ts | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/core/src/app/aggregator/aggregator.ts b/packages/core/src/app/aggregator/aggregator.ts index 0bceea64..68adddaa 100644 --- a/packages/core/src/app/aggregator/aggregator.ts +++ b/packages/core/src/app/aggregator/aggregator.ts @@ -8,7 +8,7 @@ import { Transaction, TransactionResult, } from '../../types/serialization' -import { rangesSpanRange, sign } from '../utils' +import { doRangesSpanRange, sign } from '../utils' import { BlockManager } from '../../types/block-production' export class DefaultAggregator implements Aggregator { @@ -37,7 +37,7 @@ export class DefaultAggregator implements Aggregator { witness ) - if (!rangesSpanRange(validRanges, transaction.range)) { + if (!doRangesSpanRange(validRanges, transaction.range)) { throw Error( `Cannot ingest Transaction that is not valid across its entire range. Valid Ranges: ${JSON.stringify(validRanges)}. diff --git a/packages/core/src/app/utils/equals.ts b/packages/core/src/app/utils/equals.ts index d54b644e..0df7cbdb 100644 --- a/packages/core/src/app/utils/equals.ts +++ b/packages/core/src/app/utils/equals.ts @@ -5,7 +5,6 @@ import { StateUpdate, Transaction, } from '../../types/serialization' -import { unwatchFile } from 'fs' /** * All of the below functions check whether or not the two provided objects are equal, diff --git a/packages/core/src/app/utils/range.ts b/packages/core/src/app/utils/range.ts index 6445965a..4a6a7f16 100644 --- a/packages/core/src/app/utils/range.ts +++ b/packages/core/src/app/utils/range.ts @@ -76,7 +76,7 @@ export const isRangeSubset = (subset: Range, superset: Range): boolean => { * @param rangeToSpan the Range being evaluated * @returns true if ranges span rangeToSpan, false otherwise */ -export const rangesSpanRange = ( +export const doRangesSpanRange = ( ranges: Range[], rangeToSpan: Range ): boolean => { From 9c70d450e853ded20d7ec9d014b09d624244e60a Mon Sep 17 00:00:00 2001 From: Will Meister Date: Mon, 22 Jul 2019 14:26:08 -0500 Subject: [PATCH 12/15] removoing `witness` from Aggregator.ingestTransaction(...) and updating references --- packages/core/src/app/aggregator/aggregator.ts | 5 ++--- packages/core/src/types/aggregator/aggregator.interface.ts | 4 +--- packages/core/test/app/aggregator/aggregator.spec.ts | 7 +++---- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/core/src/app/aggregator/aggregator.ts b/packages/core/src/app/aggregator/aggregator.ts index 68adddaa..c0d7f2bd 100644 --- a/packages/core/src/app/aggregator/aggregator.ts +++ b/packages/core/src/app/aggregator/aggregator.ts @@ -23,8 +23,7 @@ export class DefaultAggregator implements Aggregator { ) {} public async ingestTransaction( - transaction: Transaction, - witness: string + transaction: Transaction ): Promise { const blockNumber: BigNum = await this.blockManager.getNextBlockNumber() @@ -34,7 +33,7 @@ export class DefaultAggregator implements Aggregator { }: TransactionResult = await this.stateManager.executeTransaction( transaction, blockNumber, - witness + '' // Note: This function call will change, so just using '' so it compiles ) if (!doRangesSpanRange(validRanges, transaction.range)) { diff --git a/packages/core/src/types/aggregator/aggregator.interface.ts b/packages/core/src/types/aggregator/aggregator.interface.ts index dc2338d4..4f6930a6 100644 --- a/packages/core/src/types/aggregator/aggregator.interface.ts +++ b/packages/core/src/types/aggregator/aggregator.interface.ts @@ -5,13 +5,11 @@ export interface Aggregator { * Notifies the Aggregator of the provided Transaction so it may be included in the next block. * * @param transaction the Transaction in question - * @param witness the Witness of the Transaction in question * * @returns the BlockTransactionCommitment indicating the transaction will be included in the next block */ ingestTransaction( - transaction: Transaction, - witness: string + transaction: Transaction ): Promise /** diff --git a/packages/core/test/app/aggregator/aggregator.spec.ts b/packages/core/test/app/aggregator/aggregator.spec.ts index 01b96b77..57af1e42 100644 --- a/packages/core/test/app/aggregator/aggregator.spec.ts +++ b/packages/core/test/app/aggregator/aggregator.spec.ts @@ -123,8 +123,7 @@ describe('DefaultAggregator', () => { for (let i = 0; i < numTransactions; i++) { const txCommitment: BlockTransactionCommitment = await aggregator.ingestTransaction( - transactions[i], - '' + transactions[i] ) assert( transactionsEqual( @@ -166,7 +165,7 @@ describe('DefaultAggregator', () => { ) try { - await aggregator.ingestTransaction(undefined, '') + await aggregator.ingestTransaction(undefined) assert(false, 'This should have thrown') } catch (e) { // This is success @@ -197,7 +196,7 @@ describe('DefaultAggregator', () => { } try { - await aggregator.ingestTransaction(transaction, '') + await aggregator.ingestTransaction(transaction) assert(false, 'This should have thrown') } catch (e) { // This is success From dee7a3b5abee494f3150d64a632a061e4a080146 Mon Sep 17 00:00:00 2001 From: Will Meister Date: Mon, 22 Jul 2019 14:49:19 -0500 Subject: [PATCH 13/15] Fixing wallet nonce race condition in Deposit tests by creating a separate wallet for each async test instead of sharing --- packages/contracts/test/Deposit.spec.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/contracts/test/Deposit.spec.ts b/packages/contracts/test/Deposit.spec.ts index db2be31e..8107de7a 100644 --- a/packages/contracts/test/Deposit.spec.ts +++ b/packages/contracts/test/Deposit.spec.ts @@ -46,14 +46,19 @@ async function depositErc20( } describe('Deposit Contract with Ownership', () => { - const provider = createMockProvider() - const [wallet, walletTo] = getWallets(provider) + let provider + let wallet + let walletTo let token let depositContract let commitmentContract let ownershipPredicate beforeEach(async () => { + provider = createMockProvider() + const wallets = getWallets(provider) + wallet = wallets[0] + walletTo = wallets[1] token = await deployContract(wallet, BasicTokenMock, [wallet.address, 1000]) commitmentContract = await deployContract(wallet, Commitment, []) depositContract = await deployContract(wallet, Deposit, [ @@ -75,7 +80,7 @@ describe('Deposit Contract with Ownership', () => { }) }) - it('allows exits to be started and finalized on deposits', async () => { + it('allows exits to be started and finalized on deposits', async () => { // Deposit some money into the ownership predicate await token.approve(depositContract.address, 500) const depositData = abi.encode(['address'], [wallet.address]) From 37b9f0c80741f7b8a897cad71f9957e9e8dfe365 Mon Sep 17 00:00:00 2001 From: Will Meister Date: Mon, 22 Jul 2019 14:49:57 -0500 Subject: [PATCH 14/15] fixing linting issues --- packages/contracts/test/Deposit.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts/test/Deposit.spec.ts b/packages/contracts/test/Deposit.spec.ts index 8107de7a..7b15c928 100644 --- a/packages/contracts/test/Deposit.spec.ts +++ b/packages/contracts/test/Deposit.spec.ts @@ -80,7 +80,7 @@ describe('Deposit Contract with Ownership', () => { }) }) - it('allows exits to be started and finalized on deposits', async () => { + it('allows exits to be started and finalized on deposits', async () => { // Deposit some money into the ownership predicate await token.approve(depositContract.address, 500) const depositData = abi.encode(['address'], [wallet.address]) From 8a26d52367e1b0c0914227d345bd28ca1d91088c Mon Sep 17 00:00:00 2001 From: Will Meister Date: Mon, 22 Jul 2019 15:27:03 -0500 Subject: [PATCH 15/15] Increasing default test timeout from 2 seconds to 5 seconds --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index d4c110dd..66f26141 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "lint": "wsrun -p $(yarn --silent run pkgparse) --parallel --exclude-missing lint", "fix": "wsrun -p $(yarn --silent run pkgparse) --fast-exit --parallel --exclude-missing fix", "clean": "wsrun -p $(yarn --silent run pkgparse) -r --fast-exit --parallel --exclude-missing clean", - "test": "wsrun -p $(yarn --silent run pkgparse) --fast-exit --parallel --no-prefix --exclude-missing test", + "test": "wsrun -p $(yarn --silent run pkgparse) --fast-exit --parallel --no-prefix --exclude-missing --timeout 5000 test", "build": "lerna link && wsrun -p $(yarn --silent run pkgparse) -r --fast-exit --stages --exclude-missing build", "release": "yarn run build && lerna publish --force-publish --exact -m \"chore(@plasma-group) publish %s release\"", "release:rc": "yarn run build && lerna publish --npm-tag=rc -m \"chore(@plasma-group) publish %s release\"",