[consensus breaking] use hash_with_index(pos - 1) when checking the hash in the MMR (#1114)

* use hash_with_index(pos - 1) when checking the hash in the MMR

* fix tests, get rid of todo related to duplicate commitments
This commit is contained in:
Antioch Peverell 2018-06-20 16:11:46 -04:00 committed by GitHub
parent c9229fa97b
commit 8fee3b6922
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 6 additions and 26 deletions

View file

@ -656,17 +656,10 @@ impl<'a> Extension<'a> {
// note that this doesn't show the commitment *never* existed, just
// that this is not an existing unspent commitment right now
if let Some(hash) = self.output_pmmr.get_hash(pos) {
// processing a new fork so we may get a position on the old
// fork that exists but matches a different node
// filtering that case out
//
// TODO - the following call to hash() is *INCORRECT*
// TODO - we should be calling hash_with_index(pos - 1)
// TODO - but we cannot safely make this change in testnet2
// TODO - with this incorrect behavior we never get a match, so duplicates are
// allowed
//
if hash == OutputIdentifier::from_output(out).hash() {
// Check the hash matches what we expect.
// We may be on a fork which may result in the entry at that pos being
// different to the one we expect.
if hash == OutputIdentifier::from_output(out).hash_with_index(pos - 1) {
return Err(Error::DuplicateCommitment(commit));
}
}

View file

@ -199,23 +199,10 @@ fn test_transaction_pool_block_reconciliation() {
let mut write_pool = pool.write().unwrap();
write_pool.reconcile_block(&block).unwrap();
// TODO - this is the "correct" behavior (see below)
// assert_eq!(write_pool.total_size(), 4);
// assert_eq!(write_pool.txpool.entries[0].tx, valid_transaction);
// assert_eq!(write_pool.txpool.entries[1].tx, pool_child);
// assert_eq!(write_pool.txpool.entries[2].tx, conflict_valid_child);
// assert_eq!(write_pool.txpool.entries[3].tx, valid_child_valid);
//
// TODO - once the hash() vs hash_with_index(pos - 1) change is made in
// txhashset.apply_output() TODO - and we no longer incorrectly allow
// duplicate outputs in the MMR TODO - then this test will fail
//
assert_eq!(write_pool.total_size(), 5);
assert_eq!(write_pool.total_size(), 4);
assert_eq!(write_pool.txpool.entries[0].tx, valid_transaction);
assert_eq!(write_pool.txpool.entries[1].tx, pool_child);
assert_eq!(write_pool.txpool.entries[2].tx, conflict_valid_child);
assert_eq!(write_pool.txpool.entries[3].tx, valid_child_conflict);
assert_eq!(write_pool.txpool.entries[4].tx, valid_child_valid);
assert_eq!(write_pool.txpool.entries[3].tx, valid_child_valid);
}
}