search

mimblewimble / grin

Minimal implementation of the Mimblewimble protocol.
https://grin.mw/
Apache License 2.0
5.04k stars 991 forks source link

tx input malleability (block_hash and merkle_proof excluded from hash) #1185

Closed antiochp closed 6 years ago

antiochp commented 6 years ago

For inputs spending coinbase outputs we include a block hash and a Merkle proof in a tx input to verify "maturity". Is it possible today to silently swap a Merkle proof (or block hash) out and invalidate a tx while it is in the txpool (and propagate the now invalid version)?

https://github.com/mimblewimble/grin/blob/343904180734dc2f5af9c1a9d5965612a3abbc1d/core/src/core/transaction.rs#L665-L682

We follow a similar pattern with outputs where the rangeproof is excluded in the hash. But in the case of rangeproofs we commit to the output commitment (so the rangeproof cannot be invalidated in this way).

I don't recall if this was done intentionally for inputs or not...

ignopeverell commented 6 years ago

Doesn't ring a bell. So you're saying the transaction pool would track 2 transactions that are exactly identical, except for their Merkle proof, under the same hash?

antiochp commented 6 years ago

Actually now that I think a bit more about it - we used to do this when the txpool maintained a map of txs keyed by hash. We actually simplified this and now we're just tracking the txpool as a simple vec of txs (currently in insertion order, but this can be any order that we want to apply to the txs to prioritize them somehow in the future). But we do check for "seen this tx before" in the p2p/broadcast layer based on tx hash so a bad actor on the network could cause a lot of problems by quietly messing with txs like this.

antiochp commented 6 years ago

No longer an issue due to #1203.