Skip to content

Commit

Permalink
removes loops from getNewBlockTransactions
Browse files Browse the repository at this point in the history
when creating a new block template the mining manager collects a list of
transactions to include in the block.

the mining manager iterates over transactions in the mempool, verifies the
spends in the transactions, and tracks the total size and total fee from
included transactions.

we currently iterate over the spends in each transaction twice: once to ensure
that two transactions included in the block do not include the same spend and
again to verify each spend in the included transaction.

we also iterate over the list of included transactions twice after collecting
them: once to read the fee from the transaction and again to sum the fees.

these changes remove the extra iteration over spends and included transactions.
performance gains will likely be neglible.
  • Loading branch information
hughy committed May 23, 2023
1 parent 69ee212 commit ce6c63e
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 20 deletions.
24 changes: 18 additions & 6 deletions ironfish/src/consensus/verifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,16 +329,28 @@ export class Verifier {
if (reason) {
return { valid: false, reason }
}

if (await this.chain.nullifiers.contains(spend.nullifier, tx)) {
return { valid: false, reason: VerificationResultReason.DOUBLE_SPEND }
}
}

return { valid: true }
})
}

async verifySpend(
spend: Spend,
notesSize: number,
tx?: IDatabaseTransaction,
): Promise<VerificationResultReason | undefined> {
const reason = await this.verifySpendCommitment(spend, notesSize, tx)

if (reason) {
return reason
}

if (await this.chain.nullifiers.contains(spend.nullifier, tx)) {
return VerificationResultReason.DOUBLE_SPEND
}
}

async verifyTransactionAdd(
transaction: Transaction,
tx?: IDatabaseTransaction,
Expand Down Expand Up @@ -409,7 +421,7 @@ export class Verifier {
Assert.isNotNull(previousNotesSize)

for (const spend of block.spends()) {
const verificationError = await this.verifySpend(spend, previousNotesSize, tx)
const verificationError = await this.verifySpendCommitment(spend, previousNotesSize, tx)
if (verificationError) {
return { valid: false, reason: verificationError }
}
Expand Down Expand Up @@ -454,7 +466,7 @@ export class Verifier {
* @param notesSize the size of the notes tree
* @param tx optional transaction context within which to check the spends.
*/
async verifySpend(
async verifySpendCommitment(
spend: Spend,
notesSize: number,
tx?: IDatabaseTransaction,
Expand Down
27 changes: 13 additions & 14 deletions ironfish/src/mining/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,12 @@ export class MiningManager {
}> {
const startTime = BenchUtils.start()

const notesSize = await this.chain.notes.size()

// Fetch pending transactions
const blockTransactions: Transaction[] = []
const nullifiers = new BufferSet()
let totalTransactionFees = BigInt(0)
for (const transaction of this.memPool.orderedTransactions()) {
// Skip transactions that would cause the block to exceed the max size
const transactionSize = getTransactionSize(transaction)
Expand All @@ -85,31 +88,27 @@ export class MiningManager {
continue
}

const isConflicted = transaction.spends.find((spend) => nullifiers.has(spend.nullifier))
if (isConflicted) {
continue
}
for (const spend of transaction.spends) {
if (nullifiers.has(spend.nullifier)) {
continue
}

const { valid: isValid } = await this.chain.verifier.verifyTransactionSpends(transaction)
if (!isValid) {
continue
const reason = await this.chain.verifier.verifySpend(spend, notesSize)

if (reason) {
continue
}
}

for (const spend of transaction.spends) {
nullifiers.add(spend.nullifier)
}

currBlockSize += transactionSize
totalTransactionFees += transaction.fee()
blockTransactions.push(transaction)
}

// Sum the transaction fees
let totalTransactionFees = BigInt(0)
const transactionFees = await Promise.all(blockTransactions.map((t) => t.fee()))
for (const transactionFee of transactionFees) {
totalTransactionFees += transactionFee
}

this.metrics.mining_newBlockTransactions.add(BenchUtils.end(startTime))

return {
Expand Down

0 comments on commit ce6c63e

Please sign in to comment.