casey/runestone

Using commit_tx.confirmations to determine the validity of etching runes can lead to inconsistent judgments on the validity of runes during node operations

Closed this issue · 6 comments

alexha123123 commented

When deploying a rune, it is required that the commit tx and reveal tx for the same rune are separated by at least 6 blocks. Currently, the code logic checks the confirmations of the commit_tx using block.commit_tx.confirmations. Let's consider the scenarios for two nodes:

Node A: This node is continuously running. When it reaches block X, it executes the etching rune commit tx. Then, in block X+1, it performs the reveal tx. Since the two transactions are not separated by 6 blocks, the etching rune in this case is considered invalid.

Node B: Starting from block number 0, this node backtracks the data. When it reaches block X, it discovers the etching rune commit tx. In block X+1, it finds the reveal tx. Due to the time spent on backtracking, by the time it retrieves commit_tx.confirmations, it has already exceeded 6 blocks. Therefore, the etching rune is considered valid and appears in the list of runes.

The code in question is as follows:

https://github.com/ordinals/ord/blob/master/src/index/updater/rune_updater.rs#L399

let mature = tx_info
    .confirmations
    .map(|confirmations| confirmations >= Runestone::COMMIT_INTERVAL.into())
    .unwrap_or_default();

The suggestion is to compare the block numbers of the commit_tx and the reveal tx and check the difference to determine their separation.

casey commented

Excellent find!

casey commented

Fixed!

This would have been pretty devastating if it hadn't been fixed before launch. We don't actually have a bug bounty, but I think this definitely deserves a reward. I was thinking $1000 USD paid in bitcoin. If you feel comfortable doing so, can you drop a bitcoin address? If not, can you drop some method for me to get in touch with you so we can coordinate off GitHub?

  • 👍1
wanyvic commented

Is this issue to fix the loss of recognizing the etching of the transaction in the testnet?https://testnet.ordinals.com/tx/d484025088ddb8f4cdf4e72908608fd1cfe01d6a8d085c053939ac8ced78235e

image

casey commented

@wanyvic I'm not sure, but that's probably unrelated. Can you open a new issue with more details?

wanyvic commented

It seems that our node (version 0.17.1 and indexing from block 0) has overidentified some runes.
image

such as the reveal transaction with the hash d484025088ddb8f4cdf4e72908608fd1cfe01d6a8d085c053939ac8ced78235e in the testnet. Our node explain that it is etching a rune named CL•TEST•TUE•THREE, but the commit transaction and the reveal transaction has not been separated by at least 6 blocks actually. I believe this issue is consistent with the feedback provided by @alexha123123, so there is no need to open a new issue.

  • 👍1
alexha123123 commented

@casey Thrilled to contribute to the Runes protocol. It's truly an honor for me!
In fact, our GeniiData team has been closely monitoring the developments of Runes since September 2023, and we will continue to provide full support for the Runes protocol on our platform.
Our team would be more than happy to collaborate and co-build with you.