From d18482507a2af8ab03cf4081b56516ab305cc666 Mon Sep 17 00:00:00 2001 From: Christopher Jeffrey Date: Mon, 19 Sep 2016 06:53:21 -0700 Subject: [PATCH] mempool: safer handling of wtxs. --- lib/mempool/mempool.js | 225 +++++++++++++++++++++++++++++++------- lib/net/pool.js | 75 ++++++------- lib/primitives/tx.js | 134 +++++++++++++++-------- lib/protocol/constants.js | 9 ++ lib/utils/errors.js | 1 + 5 files changed, 317 insertions(+), 127 deletions(-) diff --git a/lib/mempool/mempool.js b/lib/mempool/mempool.js index d1bc9237..c6c7b480 100644 --- a/lib/mempool/mempool.js +++ b/lib/mempool/mempool.js @@ -17,6 +17,7 @@ var BufferReader = require('../utils/reader'); var crypto = require('../crypto/crypto'); var VerifyError = bcoin.errors.VerifyError; var VerifyResult = utils.VerifyResult; +var flags = constants.flags; /** * Represents a mempool. @@ -85,6 +86,8 @@ function Mempool(options) { this.coinIndex = new AddressIndex(this); this.txIndex = new AddressIndex(this); + this.rejects = new bcoin.bloom.rolling(120000, 0.000001); + this.freeCount = 0; this.lastTime = 0; @@ -190,6 +193,10 @@ Mempool.prototype.addBlock = function addBlock(block, callback) { if (this.fees) this.fees.processBlock(block.height, entries, this.chain.isFull()); + // We need to reset the rejects filter periodically. + // There may be a locktime in a TX that is now valid. + this.rejects.reset(); + utils.nextTick(callback); }; @@ -209,6 +216,8 @@ Mempool.prototype.removeBlock = function removeBlock(block, callback) { if (!callback) return; + this.rejects.reset(); + utils.forEachSerial(block.txs, function(tx, next) { var hash = tx.hash('hex'); @@ -490,7 +499,8 @@ Mempool.prototype.hasTX = function hasTX(hash) { }; /** - * Test the mempool to see if it contains a transaction or an orphan. + * Test the mempool to see if it + * contains a transaction or an orphan. * @param {Hash} hash * @returns {Boolean} */ @@ -505,6 +515,17 @@ Mempool.prototype.has = function has(hash) { return this.hasTX(hash); }; +/** + * Test the mempool to see if it + * contains a recent reject. + * @param {Hash} hash + * @returns {Boolean} + */ + +Mempool.prototype.hasReject = function hasReject(hash) { + return this.rejects.test(hash, 'hex'); +}; + /** * Add a transaction to the mempool. Note that this * will lock the mempool until the transaction is @@ -514,12 +535,35 @@ Mempool.prototype.has = function has(hash) { */ Mempool.prototype.addTX = function addTX(tx, callback) { + var self = this; + this._addTX(tx, function(err, missing) { + if (err) { + if (err.type === 'VerifyError') { + if (!tx.hasWitness() && !err.malleated) + self.rejects.add(tx.hash()); + + return callback(err); + } + return callback(err); + } + callback(null, missing); + }); +}; + +/** + * Add a transaction to the mempool. + * @private + * @param {TX} tx + * @param {Function} callback - Returns [{@link VerifyError}]. + */ + +Mempool.prototype._addTX = function _addTX(tx, callback) { var self = this; var lockFlags = constants.flags.STANDARD_LOCKTIME_FLAGS; var hash = tx.hash('hex'); - var ret, entry; + var ret, entry, missing; - callback = this._lock(addTX, [tx, callback]); + callback = this._lock(_addTX, [tx, callback]); if (!callback) return; @@ -619,8 +663,8 @@ Mempool.prototype.addTX = function addTX(tx, callback) { return callback(err); if (!tx.hasCoins()) { - self.storeOrphan(tx); - return callback(); + missing = self.storeOrphan(tx); + return callback(null, missing); } entry = MempoolEntry.fromTX(tx, self.chain.height); @@ -687,7 +731,10 @@ Mempool.prototype.addUnchecked = function addUnchecked(entry, callback, force) { self.logger.debug('Could not resolve orphan %s: %s.', tx.rhash, err.message); - self.emit('bad orphan', tx, entry); + + if (!tx.hasWitness() && !err.malleated) + self.rejects.add(tx.hash()); + return next(); } self.emit('error', err); @@ -792,18 +839,15 @@ Mempool.prototype.getMinRate = function getMinRate() { Mempool.prototype.verify = function verify(entry, callback) { var self = this; var height = this.chain.height + 1; - var lockFlags = constants.flags.STANDARD_LOCKTIME_FLAGS; - var flags = constants.flags.STANDARD_VERIFY_FLAGS; - var mandatory = constants.flags.MANDATORY_VERIFY_FLAGS; + var lockFlags = flags.STANDARD_LOCKTIME_FLAGS; + var flags1 = flags.STANDARD_VERIFY_FLAGS; + var flags2 = flags1 & ~(flags.VERIFY_WITNESS | flags.VERIFY_CLEANSTACK); + var flags3 = flags1 & ~flags.VERIFY_CLEANSTACK; + var mandatory = flags.MANDATORY_VERIFY_FLAGS; var tx = entry.tx; var ret = new VerifyResult(); var fee, modFee, now, size, rejectFee, minRelayFee, minRate, count; - if (this.chain.state.hasWitness()) - mandatory |= constants.flags.VERIFY_WITNESS; - else - flags &= ~constants.flags.VERIFY_WITNESS; - this.checkLocks(tx, lockFlags, function(err, result) { if (err) return callback(err); @@ -823,11 +867,13 @@ Mempool.prototype.verify = function verify(entry, callback) { 0)); } // if (self.chain.state.hasWitness()) { - // if (!tx.hasStandardWitness()) { - // return callback(new VerifyError(tx, + // if (!tx.hasStandardWitness(true, ret)) { + // ret = new VerifyError(tx, // 'nonstandard', - // 'bad-witness-nonstandard', - // 0)); + // ret.reason, + // ret.score); + // ret.malleated = ret.score > 0; + // return callback(ret); // } // } } @@ -905,31 +951,114 @@ Mempool.prototype.verify = function verify(entry, callback) { if (!tx.checkInputs(height, ret)) return callback(new VerifyError(tx, 'invalid', ret.reason, ret.score)); - // Do this in the worker pool. + // Standard verification + self.checkInputs(tx, flags1, function(error) { + if (!error) { + return self.checkResult(tx, mandatory, function(err, result) { + if (err) { + assert(err.type !== 'VerifyError', + 'BUG: Verification failed for mandatory but not standard.'); + return callback(err); + } + callback(); + }); + } + + if (error.type !== 'VerifyError') + return callback(error); + + if (tx.hasWitness()) + return callback(error); + + // Try without segwit and cleanstack. + self.checkResult(tx, flags2, function(err, result) { + if (err) + return callback(err); + + // If it failed, the first verification + // was the only result we needed. + if (!result) + return callback(error); + + // If it succeeded, segwit may be causing the + // failure. Try with segwit but without cleanstack. + self.checkResult(tx, flags3, function(err, result) { + if (err) + return callback(err); + + // Cleanstack was causing the failure. + if (result) + return callback(error); + + // Do not insert into reject cache. + error.malleated = true; + callback(error); + }); + }); + }); + }); +}; + +/** + * Verify inputs, return a boolean + * instead of an error based on success. + * @param {TX} tx + * @param {VerifyFlags} flags + * @param {Function} callback + */ + +Mempool.prototype.checkResult = function checkResult(tx, flags, callback) { + this.checkInputs(tx, flags, function(err) { + if (err) { + if (err.type === 'VerifyError') + return callback(null, false); + return callback(err); + } + callback(null, true); + }); +}; + +/** + * Verify inputs for standard + * _and_ mandatory flags on failure. + * @param {TX} tx + * @param {VerifyFlags} flags + * @param {Function} callback + */ + +Mempool.prototype.checkInputs = function checkInputs(tx, flags, callback) { + // Do this in the worker pool. + tx.verifyAsync(flags, function(err, result) { + if (err) + return callback(err); + + if (result) + return callback(); + + if (!(flags & constants.flags.UNSTANDARD_VERIFY_FLAGS)) { + return callback(new VerifyError(tx, + 'nonstandard', + 'non-mandatory-script-verify-flag', + 0)); + } + + flags &= ~constants.flags.UNSTANDARD_VERIFY_FLAGS; + tx.verifyAsync(flags, function(err, result) { if (err) return callback(err); - if (!result) { - return tx.verifyAsync(mandatory, function(err, result) { - if (err) - return callback(err); - - if (result) { - return callback(new VerifyError(tx, - 'nonstandard', - 'non-mandatory-script-verify-flag', - 0)); - } - - return callback(new VerifyError(tx, - 'nonstandard', - 'mandatory-script-verify-flag', - 0)); - }); + if (result) { + return callback(new VerifyError(tx, + 'nonstandard', + 'non-mandatory-script-verify-flag', + 0)); } - callback(); + return callback(new VerifyError(tx, + 'nonstandard', + 'mandatory-script-verify-flag', + 100)); }); }); }; @@ -1064,14 +1193,30 @@ Mempool.prototype.getDepends = function getDepends(tx) { Mempool.prototype.storeOrphan = function storeOrphan(tx) { var prevout = {}; + var missing = []; var i, hash, input, prev; - if (tx.getSize() > 99999) { + if (tx.getWeight() > constants.tx.MAX_WEIGHT) { this.logger.debug('Ignoring large orphan: %s', tx.rhash); - this.emit('bad orphan', tx); + if (!tx.hasWitness()) + this.rejects.add(tx.hash()); return; } + for (i = 0; i < tx.inputs.length; i++) { + input = tx.inputs[i]; + + if (input.coin) + continue; + + if (this.hasReject(input.prevout.hash)) { + this.logger.debug('Not storing orphan %s (rejected parents).', tx.rhash); + return; + } + + missing.push(input.prevout.hash); + } + hash = tx.hash('hex'); for (i = 0; i < tx.inputs.length; i++) { @@ -1099,6 +1244,8 @@ Mempool.prototype.storeOrphan = function storeOrphan(tx) { this.emit('add orphan', tx); this.limitOrphans(); + + return missing; }; /** diff --git a/lib/net/pool.js b/lib/net/pool.js index 0d821588..b95939f2 100644 --- a/lib/net/pool.js +++ b/lib/net/pool.js @@ -124,8 +124,6 @@ function Pool(options) { this.spvFilter = null; this.txFilter = null; - this.rejectsFilter = new bcoin.bloom.rolling(120000, 0.000001); - this.rejectsTip = null; // Requested objects. this.requestMap = {}; @@ -243,12 +241,6 @@ Pool.prototype._initOptions = function _initOptions() { Pool.prototype._init = function _init() { var self = this; - if (this.mempool) { - this.mempool.on('bad orphan', function(tx) { - self.rejectsFilter.add(tx.hash()); - }); - } - this.chain.on('block', function(block, entry) { self.emit('block', block, entry); }); @@ -764,17 +756,12 @@ Pool.prototype._handleHeaders = function _handleHeaders(headers, peer, callback) var hash = header.hash('hex'); if (last && header.prevBlock !== last) { - // Note: We do _not_ want to add this - // to known rejects. This block may - // very well be valid, but this peer - // is being an asshole right now. peer.setMisbehavior(100); return next(new Error('Bad header chain.')); } if (!header.verify(ret)) { peer.reject(header, 'invalid', ret.reason, 100); - self.rejectsFilter.add(header.hash()); return next(new Error('Invalid header.')); } @@ -958,7 +945,6 @@ Pool.prototype._handleBlock = function _handleBlock(block, peer, callback) { }); } - self.rejectsFilter.add(block.hash()); self.scheduleRequests(peer); return callback(err); } @@ -1262,9 +1248,6 @@ Pool.prototype.createPeer = function createPeer(addr, socket) { Pool.prototype._handleAlert = function _handleAlert(alert, peer) { var now = bcoin.now(); - if (!this.rejectsFilter.added(alert.hash())) - return; - if (!alert.verify(this.network.alertKey)) { this.logger.warning('Peer sent a phony alert packet (%s).', peer.hostname); // Let's look at it because why not? @@ -1315,6 +1298,19 @@ Pool.prototype._handleAlert = function _handleAlert(alert, peer) { this.emit('alert', alert, peer); }; +/** + * Test the mempool to see if it + * contains a recent reject. + * @param {Hash} hash + * @returns {Boolean} + */ + +Pool.prototype.hasReject = function hasReject(hash) { + if (!this.mempool) + return false; + return this.mempool.hasReject(hash); +}; + /** * Handle a transaction. Attempt to add to mempool. * @private @@ -1325,7 +1321,7 @@ Pool.prototype._handleAlert = function _handleAlert(alert, peer) { Pool.prototype._handleTX = function _handleTX(tx, peer, callback) { var self = this; - var requested; + var i, requested; callback = utils.ensure(callback); @@ -1341,10 +1337,10 @@ Pool.prototype._handleTX = function _handleTX(tx, peer, callback) { this.logger.warning('Peer sent unrequested tx: %s (%s).', tx.rhash, peer.hostname); - if (this.rejectsFilter.test(tx.hash())) { + if (this.hasReject(tx.hash())) { return callback(new VerifyError(tx, 'alreadyknown', - 'txn-already-known', + 'txn-already-in-mempool', 0)); } } @@ -1354,23 +1350,21 @@ Pool.prototype._handleTX = function _handleTX(tx, peer, callback) { return callback(); } - this.mempool.addTX(tx, function(err) { + this.mempool.addTX(tx, function(err, missing) { if (err) { if (err.type === 'VerifyError') { if (err.score !== -1) peer.reject(tx, err.code, err.reason, err.score); - - // Once we test it more, we can do: - // if (err.reason !== 'bad-witness-nonstandard') - - if (!tx.hasWitness()) - self.rejectsFilter.add(tx.hash()); - return callback(err); } return callback(err); } + if (missing) { + for (i = 0; i < missing.length; i++) + self.getData(peer, self.txType, missing[i]); + } + self.emit('tx', tx, peer); callback(); @@ -1551,7 +1545,7 @@ Pool.prototype.getData = function getData(peer, type, hash, callback) { if (!this.loaded) return callback(); - this.has(type, hash, function(err, exists) { + this.has(peer, type, hash, function(err, exists) { if (err) return callback(err); @@ -1587,12 +1581,13 @@ Pool.prototype.getData = function getData(peer, type, hash, callback) { /** * Test whether the pool has or has seen an item. + * @param {Peer} peer * @param {InvType} type * @param {Hash} hash * @param {Function} callback - Returns [Error, Boolean]. */ -Pool.prototype.has = function has(type, hash, callback) { +Pool.prototype.has = function has(peer, type, hash, callback) { var self = this; this.exists(type, hash, function(err, exists) { @@ -1606,17 +1601,15 @@ Pool.prototype.has = function has(type, hash, callback) { if (self.requestMap[hash]) return callback(null, true); - // We need to reset the rejects filter periodically. - // There may be a locktime in a TX that is now valid. - if (self.rejectsTip !== self.chain.tip.hash) { - self.rejectsTip = self.chain.tip.hash; - self.rejectsFilter.reset(); - } else { - // If we recently rejected this item. Ignore. - if (self.rejectsFilter.test(hash, 'hex')) { - self.logger.spam('Peer sent a known reject: %s.', utils.revHex(hash)); - return callback(null, true); - } + if (type !== self.txType) + return callback(null, false); + + // If we recently rejected this item. Ignore. + if (self.hasReject(hash)) { + self.logger.spam( + 'Peer sent a known reject of %s (%s).', + utils.revHex(hash), peer.hostname); + return callback(null, true); } return callback(null, false); diff --git a/lib/primitives/tx.js b/lib/primitives/tx.js index 74234be5..823555aa 100644 --- a/lib/primitives/tx.js +++ b/lib/primitives/tx.js @@ -17,6 +17,15 @@ var Stack = bcoin.stack; var BufferWriter = require('../utils/writer'); var VerifyResult = utils.VerifyResult; +/* + * Constants + */ + +var BAD_OKAY = 0; +var BAD_WITNESS = 1; +var BAD_P2SH = 2; +var BAD_NONSTD_P2WSH = 3; + /** * A static transaction object. * @exports TX @@ -1365,11 +1374,51 @@ TX.prototype.hasStandardInputs = function hasStandardInputs() { * @returns {Boolean} */ -TX.prototype.hasStandardWitness = function hasStandardWitness(strict) { +TX.prototype.hasStandardWitness = function hasStandardWitness(strict, ret) { + var result; + + if (!ret) + ret = new VerifyResult(); + + result = this._hasStandardWitness(); + + switch (result) { + case BAD_WITNESS: + ret.reason = 'bad-witness'; + ret.score = 100; + return false; + case BAD_P2SH: + ret.reason = 'bad-P2SH-scriptSig'; + ret.score = 100; + return false; + case BAD_NONSTD_P2WSH: + if (strict) { + ret.reason = 'bad-witness-nonstandard'; + ret.score = 0; + return false; + } + return true; + } + + return true; +}; + +/** + * Perform contextual checks to verify coin and witness standardness. + * @private + * @see IsBadWitness() + * @returns {Boolean} + */ + +TX.prototype._hasStandardWitness = function _hasStandardWitness() { + var ret = BAD_OKAY; var i, j, input, prev, hash, redeem, m, n; + if (!this.hasWitness()) + return ret; + if (this.isCoinbase()) - return true; + return ret; for (i = 0; i < this.inputs.length; i++) { input = this.inputs[i]; @@ -1377,31 +1426,31 @@ TX.prototype.hasStandardWitness = function hasStandardWitness(strict) { if (!input.coin) continue; + if (input.witness.length === 0) + continue; + prev = input.coin.script; if (prev.isScripthash()) { prev = input.script.getRedeem(); if (!prev) - return false; + return BAD_P2SH; } - if (!prev.isProgram()) { - if (input.witness.length !== 0) - return false; - continue; - } + if (!prev.isProgram()) + return BAD_WITNESS; if (prev.isWitnessPubkeyhash()) { if (input.witness.length !== 2) - return false; + return BAD_WITNESS; if (input.witness.get(0).length > 73) - return false; + return BAD_WITNESS; hash = crypto.hash160(input.witness.get(1)); if (!utils.equal(hash, prev.get(1))) - return false; + return BAD_WITNESS; continue; } @@ -1412,61 +1461,52 @@ TX.prototype.hasStandardWitness = function hasStandardWitness(strict) { continue; } - if (input.witness.length === 0) - return false; - - // Based on Johnson Lau's calculations: - if (input.witness.length - 1 > 604) - return false; - - if (strict) { - if (input.witness.length - 1 > constants.script.MAX_P2WSH_STACK) - return false; - } - - for (j = 0; j < input.witness.length; j++) { - if (input.witness.get(j).length > constants.script.MAX_PUSH) - return false; - - if (strict) { - if (input.witness.get(j).length > constants.script.MAX_P2WSH_PUSH) - return false; - } - } - redeem = input.witness.get(input.witness.length - 1); if (redeem.length > constants.script.MAX_SIZE) - return false; + return BAD_WITNESS; - if (strict) { - if (redeem.length > constants.script.MAX_P2WSH_SIZE) - return false; - } + if (redeem.length > constants.script.MAX_P2WSH_SIZE) + ret = BAD_NONSTD_P2WSH; hash = crypto.sha256(redeem); if (!utils.equal(hash, prev.get(1))) - return false; + return BAD_WITNESS; + + // Based on Johnson Lau's calculations: + if (input.witness.length - 1 > 604) + return BAD_WITNESS; + + if (input.witness.length - 1 > constants.script.MAX_P2WSH_STACK) + ret = BAD_NONSTD_P2WSH; + + for (j = 0; j < input.witness.length; j++) { + if (input.witness.get(j).length > constants.script.MAX_PUSH) + return BAD_WITNESS; + + if (input.witness.get(j).length > constants.script.MAX_P2WSH_PUSH) + ret = BAD_NONSTD_P2WSH; + } redeem = new bcoin.script(redeem); if (redeem.isPubkey()) { if (input.witness.length - 1 !== 1) - return false; + return BAD_WITNESS; if (input.witness.get(0).length > 73) - return false; + return BAD_WITNESS; continue; } if (redeem.isPubkeyhash()) { if (input.witness.length - 1 !== 2) - return false; + return BAD_WITNESS; if (input.witness.get(0).length > 73) - return false; + return BAD_WITNESS; continue; } @@ -1476,19 +1516,19 @@ TX.prototype.hasStandardWitness = function hasStandardWitness(strict) { n = redeem.getSmall(redeem.length - 2); if (input.witness.length - 1 !== m + 1) - return false; + return BAD_WITNESS; if (input.witness.get(0).length !== 0) - return false; + return BAD_WITNESS; for (j = 1; j < input.witness.length - 1; j++) { if (input.witness.get(i).length > 73) - return false; + return BAD_WITNESS; } } } - return true; + return ret; }; /** diff --git a/lib/protocol/constants.js b/lib/protocol/constants.js index e44622be..dadbb30f 100644 --- a/lib/protocol/constants.js +++ b/lib/protocol/constants.js @@ -809,6 +809,15 @@ exports.flags.STANDARD_VERIFY_FLAGS = 0 | exports.flags.VERIFY_WITNESS | exports.flags.VERIFY_DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM; +/** + * Standard-not-mandatory flags. + * @const {VerifyFlags} + * @default + */ + +exports.flags.UNSTANDARD_VERIFY_FLAGS = + exports.flags.STANDARD_VERIFY_FLAGS & ~exports.flags.MANDATORY_VERIFY_FLAGS; + /** * Consensus locktime flags (used for block validation). * @const {LockFlags} diff --git a/lib/utils/errors.js b/lib/utils/errors.js index 9f553ba7..f8067eb0 100644 --- a/lib/utils/errors.js +++ b/lib/utils/errors.js @@ -51,6 +51,7 @@ function VerifyError(msg, code, reason, score) { this.code = code; this.reason = score === -1 ? null : reason; this.score = score; + this.malleated = false; this.message = 'Verification failure: ' + reason + ' (code=' + code