Skip to content
This repository has been archived by the owner on Apr 15, 2024. It is now read-only.

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

Closed
alexha123123 opened this issue Apr 8, 2024 · 6 comments

Comments

@alexha123123
Copy link

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
Copy link
Owner

casey commented Apr 8, 2024

Excellent find!

@casey
Copy link
Owner

casey commented Apr 10, 2024

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?

@casey casey closed this as completed Apr 10, 2024
@wanyvic
Copy link

wanyvic commented Apr 11, 2024

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
Copy link
Owner

casey commented Apr 11, 2024

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

@wanyvic
Copy link

wanyvic commented Apr 11, 2024

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.

@alexha123123
Copy link
Author

@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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants