Skip to content

[DNM] https://github.com/mimblewimble/grin/issues/3620 #3621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions chain/tests/mine_simple_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,89 @@ fn output_header_mappings() {
clean_output_dir(".grin_header_for_output");
}

/// Test the duplicate rangeproof bug
#[test]
fn test_duplicate_rangeproof() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

clean_output_dir(".grin_overflow");
global::set_local_chain_type(ChainTypes::AutomatedTesting);
util::init_test_logger();
{
let chain = init_chain(".grin_overflow", pow::mine_genesis_block().unwrap());
let prev = chain.head_header().unwrap();
let kc = ExtKeychain::from_random_seed(false).unwrap();
let pb = ProofBuilder::new(&kc);

let mut head = prev;

// mine the first block and keep track of the block_hash
// so we can spend the coinbase later
let b = prepare_block(&kc, &head, &chain, 2);

assert!(b.outputs()[0].is_coinbase());
head = b.header.clone();
chain
.process_block(b.clone(), chain::Options::SKIP_POW)
.unwrap();

// now mine three further blocks
for n in 3..6 {
let b = prepare_block(&kc, &head, &chain, n);
head = b.header.clone();
chain.process_block(b, chain::Options::SKIP_POW).unwrap();
}

let key_id2 = ExtKeychainPath::new(1, 2, 0, 0, 0).to_identifier();
let key_id30 = ExtKeychainPath::new(1, 30, 0, 0, 0).to_identifier();
let key_id31 = ExtKeychainPath::new(1, 31, 0, 0, 0).to_identifier();
let key_id32 = ExtKeychainPath::new(1, 32, 0, 0, 0).to_identifier();

// build a regular transaction so we have a rangeproof to copy
let tx1 = build::transaction(
KernelFeatures::Plain { fee: 20000.into() },
&[
build::coinbase_input(consensus::REWARD, key_id2.clone()),
build::output(consensus::REWARD - 20000, key_id30.clone()),
],
&kc,
&pb,
)
.unwrap();

// mine block with tx1
let next = prepare_block_tx(&kc, &head, &chain, 7, &[tx1.clone()]);
let prev_main = next.header.clone();
chain
.process_block(next.clone(), chain::Options::SKIP_POW)
.unwrap();
chain.validate(false).unwrap();

// create a second tx
let mut tx2 = build::transaction(
KernelFeatures::Plain { fee: 0.into() },
&[
build::input(consensus::REWARD - 20000, key_id30.clone()), // 59999980000
build::output(consensus::REWARD - 20001, key_id31.clone()),
build::output(1, key_id32.clone()),
],
&kc,
&pb,
)
.unwrap();

// overwrite our rangeproof with a rangeproof from last block
tx2.body.outputs[1].proof = tx1.body.outputs[0].proof;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to ensure we are explicit in which rangeproof is being replaced and which one we are replacing it with. The order (outputs[1] etc.) will not necessarily be consistent and we may accidentally pick one that would cause the validation to fail for unrelated reasons (swapping out a coinbase one for example).


let next = prepare_block_tx(&kc, &prev_main, &chain, 8, &[tx2.clone()]);
// process_block fails with verifier_cache disabled or with correct verifier_cache
// implementations
assert_eq!(
chain.process_block(next, chain::Options::SKIP_POW).is_ok(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just merged #3616 to master and we now propagate up a more useful error on failure here.
We should test for the error explicitly as part of this test - it should be an InvalidBlockProof.
We don't want to accidentally have this test passing because of some unrelated error occurring during block processing.

false
);
}
clean_output_dir(".grin_overflow");
}

// Use diff as both diff *and* key_idx for convenience (deterministic private key for test blocks)
fn prepare_block<K>(kc: &K, prev: &BlockHeader, chain: &Chain, diff: u64) -> Block
where
Expand Down