Skip to content
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

Inscription numbers off by one, or the curious case of transaction c1e0db6368a43...d9e4117151 #2062

Closed
lgalabru opened this issue May 4, 2023 · 59 comments

Comments

@lgalabru
Copy link

lgalabru commented May 4, 2023

Block 788200 is including a curious transaction

Inputs:

2814d0a3c9e6dd5b88911a6280fc3899391f5c47072eb11593af2838160fad2f:0 : 0 sats

Outputs:

bc1q6zpf4gefu4ckuud3pjch563nm7x27u4r5j4ua7 : 0 sats

No satoshi at play in this transaction. Yet, ord validated the inscription (3492721) attached to the input https://ordinals.com/tx/c1e0db6368a43f5589352ed44aa1ff9af33410e4a9fd9be0f6ac42d9e4117151
which sounds like a bug.

Philosophically, the satoshi inscribed was transferred to the miner as a transaction fee, but was nevertheless inscribed by its previous owner.

Even if the miner own this UTXO, this is not how they should be inscribing it, per the protocol (they should be inscribing the coinbase transaction).

As a consequence, all the satoshis inscribed after inscription 3492720 are off by one.

@lgalabru lgalabru changed the title Inscription numbers off by one, or the curious case of transaction c1e0db6368a43f5589352ed44aa1ff9af33410e4a9fd9be0f6ac42d9e4117151 Inscription numbers off by one, or the curious case of transaction c1e0db6368a43...d9e4117151 May 4, 2023
@diwakergupta
Copy link

If this is intended behavior, it sounds like a violation of the spec (implicitly or explicitly): one should not be able to inscribe a sat that one does not own.

If this is unintended behavior, then this bug likely impacts all Ordinal tools, explorers and marketplaces out there.

@casey
Copy link
Collaborator

casey commented May 4, 2023

A curious transaction indeed! I'm told this was found by supertestnet, so nice find by them!

It shouldn't be possible to inscribe sats that you don't own, so this is a bug. However, fixing the bug by making ord ignore this inscription would change inscription numbers after the curious transaction. I'm honestly not sure what to do!

@lgalabru
Copy link
Author

lgalabru commented May 4, 2023

Yeah, apparently supertestnet initiated this transaction.
I discovered this issue the hard way and led my own investigation, my re-implementation of the ordinal protocol was stuck on this transaction, blocking our explorer.
Fixing this issue sounds like a sound approach, this bug is a violation of what crypto stands for.
Not your keys, not your sats, and no exception.

@supertestnet
Copy link

I am become Math, breaker of jpegs

@janniks
Copy link

janniks commented May 4, 2023

... fixing the bug by making ord ignore this inscription would change inscription numbers after the curious transaction. I'm honestly not sure what to do!

Might be a good issue/time for a precedent of preferring correctness over stable inscription numbers. Similar to as with ordinal theory if there was a bug in a counting implementation one would very likely favor correctness.
But yes, hard choice to make -- very subjective 😬

@supertestnet
Copy link

So if I do this 545 more times will a bunch of brc20 tokens suddenly belong to the wrong people?

@casey
Copy link
Collaborator

casey commented May 4, 2023

So if I do this 545 more times will a bunch of brc20 tokens suddenly belong to the wrong people?

God I hope so!

@DeCryptoPal
Copy link

FAAFO everyday !

@BennyTheDev
Copy link

I got one case where it doesn't show up in the explorers but the ord wallet treats it as regular ordinal. not sure if related, but I cannot tell what number it got.

@cryptoquick
Copy link
Contributor

Philosophically, the satoshi inscribed was transferred to the miner as a transaction fee, but was nevertheless inscribed by its previous owner.

One thing that bears mentioning is, no fee was paid either, at least, on-chain. This is a PHANTOM SAT

@supertestnet
Copy link

I released a tool for increasing the off-by-one error: https://github.com/supertestnet/breaker-of-jpegs

Please follow the installation and usage instructions and let me know if it works for you

@gmart7t2
Copy link
Contributor

gmart7t2 commented May 4, 2023

Similar to as with ordinal theory if there was a bug in a counting implementation one would very likely favor correctness.

There was, and we did.

Issue #1841 caused hundreds of inscriptions to be tracked incorrectly. It was fixed by #1971 which fixes the tracked location of those inscriptions.

The fix requires the index.redb file to be recreated, but the release notes didn't mention that fact so most people are probably still working with incorrect inscription locations, so I don't see why this would be any different. The indexer was incorrectly recognizing inscriptions that it shouldn't, and it needs fixing.

I would prefer if the release notes mention that a fix was applied and what the impact was if it makes a significant difference.

@kPatch
Copy link

kPatch commented May 4, 2023

Happy Ordinals Breaking Day!

@dannydeezy
Copy link

it seems this affects "inscription numbers".. does it also affect satoshi ordinal tracking (satoshi numbers) - note these are different things and i hope its just the former

@dannydeezy
Copy link

if its just "inscription numbers" then good, i think inscription numbers should have died a long time ago.
if all of ordinal tracking is affected, then my rare sat bizniss is ded!!!

@Guerrillabitcoin
Copy link

GUERRILLA WAS HERE

@RedPurdy
Copy link

RedPurdy commented May 4, 2023

RED PURDY WAS HERE

@rot13maxi
Copy link
Contributor

I think the only off-by-one here is that an inscription in the index and associated with the wrong sat, so if that inscription shouldn't have been indexed (because there is no sat in the first output), then the inscription numbers after this one are off-by-one.

AFAIU, there shouldn't be any impact to ordinal tracking, only to inscription numbers after this one.

@ottosch
Copy link

ottosch commented May 4, 2023

Bro, you have increased the UTXO set with a (likely) unspendable output:

$ bitcoin-cli gettxout c1e0db6368a43f5589352ed44aa1ff9af33410e4a9fd9be0f6ac42d9e4117151 0
{
  "bestblock": "00000000000000000003df2c062bba629e2881e8c96ce604fcedc56b0f197370",
  "confirmations": 97,
  "value": 0.00000000,
  "scriptPubKey": {
    "asm": "0 d0829aa329e5716e71b10cb17a6a33df8caf72a3",
    "desc": "addr(bc1q6zpf4gefu4ckuud3pjch563nm7x27u4r5j4ua7)#hpavz6rt",
    "hex": "0014d0829aa329e5716e71b10cb17a6a33df8caf72a3",
    "address": "bc1q6zpf4gefu4ckuud3pjch563nm7x27u4r5j4ua7",
    "type": "witness_v0_keyhash"
  },
  "coinbase": false
}

@hydren-crypto
Copy link

@0xSuku
Copy link

0xSuku commented May 4, 2023

Stop breaking things

@dannydeezy
Copy link

i attempted to summarize the situation here, lmk if anything is wrong!
https://twitter.com/dannydiekroeger/status/1654259984375615490?s=20

@KDDavis91
Copy link

you will use soma and you will like it

@dannydeezy
Copy link

you will use soma and you will like it

ahhh i'm somaaaing

@utxo-one
Copy link

utxo-one commented May 5, 2023

Running breaker of jpegs until degeneracy improves.

@satoshi0770
Copy link

I'm honestly not sure what to do!

First I think we should test all possible edge cases, to see how the indexer currently treats it.
example1: what if the tx had a 2nd output? would the indexer treat the first sat of the 2nd output, as the inscribed sat?
example2: what if this transaction was the last one in a block? would the crash the indexer? would the indexer ascribe it to the first sat in the coinbase? or would it index it to the first sat in the next block?

If nothing too weird happens after testing all possible cases. For example indexer doesn't crash, or if it doesn't ascribe to the next blocks sats. ect.

Then we can update the specs to the exact logic the indexer uses, and its fine to continue.

One is not really inscribing to a sat they don't own, as this type of inscription (if we end up counting it as one), can only be done if a miner wants to include it. (its nonstandard and default mempool policy nodes dont relay this).
So a miner is choosing to inscribe to their own sats.

TLDR: we need to test all possible cases, and its potentially a nothing burger just would need update the spec to its exact logic.

@gmart7t2
Copy link
Contributor

gmart7t2 commented May 5, 2023

The fix is as simple as this:

diff --git a/src/index/updater/inscription_updater.rs b/src/index/updater/inscription_updater.rs
index 1c18c7f..6c6c4a6 100644
--- a/src/index/updater/inscription_updater.rs
+++ b/src/index/updater/inscription_updater.rs
@@ -108,7 +108,7 @@ impl<'a, 'db, 'tx> InscriptionUpdater<'a, 'db, 'tx> {
       }
     }
 
-    if inscriptions.iter().all(|flotsam| flotsam.offset != 0)
+    if input_value != 0 && inscriptions.iter().all(|flotsam| flotsam.offset != 0)
       && Inscription::from_transaction(tx).is_some()
     {
       inscriptions.push(Flotsam {

That is, only recognize inscriptions if there is at least 1 input sat, because then there will be a sat to inscribe on.

@gmart7t2
Copy link
Contributor

gmart7t2 commented May 5, 2023

I tested doing this trick in the last tx of a block and the inscription's location was set to 00000000:-1 as expected. It didn't crash the indexer.

@jotapea
Copy link

jotapea commented May 5, 2023

I'm honestly not sure what to do!

@casey what about these becoming the "official" Vanilla inscriptions? I'm sure you can come up with a much cooler name though 😆.

And in all seriousness, I believe this can be tied to deprioritizing inscription numbers. Is still early!

@boring877
Copy link

boring877 commented May 5, 2023

So if I do this 545 more times will a bunch of brc20 tokens suddenly belong to the wrong people?

image

Brc 20 has been made https://unisat.io/brc20/%20545

{"p":"brc-20","op":"mint","tick":" 545","amt":"5000"}
fdf8420d4ebe2101481e3f3f450729fa

@drynooo
Copy link

drynooo commented May 5, 2023

So does anyone know where the code is that is causing the problem?

@wanyvic
Copy link

wanyvic commented May 12, 2023

The fix is as simple as this:

diff --git a/src/index/updater/inscription_updater.rs b/src/index/updater/inscription_updater.rs
index 1c18c7f..6c6c4a6 100644
--- a/src/index/updater/inscription_updater.rs
+++ b/src/index/updater/inscription_updater.rs
@@ -108,7 +108,7 @@ impl<'a, 'db, 'tx> InscriptionUpdater<'a, 'db, 'tx> {
       }
     }
 
-    if inscriptions.iter().all(|flotsam| flotsam.offset != 0)
+    if input_value > 0 && inscriptions.iter().all(|flotsam| flotsam.offset != 0)
       && Inscription::from_transaction(tx).is_some()
     {
       inscriptions.push(Flotsam {

That is, only recognize inscriptions if there is at least 1 input sat, because then there will be a sat to inscribe on.

greater than zero might be better

@casey
Copy link
Collaborator

casey commented May 12, 2023

I think the thing to do is to recognize the inscription, thus not throwing off inscription numbers, but treat it as having no location, i.e. as having not been made on any particular sat, and not having a location, output, or offset on the /inscription page.

@lgalabru
Copy link
Author

I think I'm in the camp of gradually decreasing the importance of inscription number. They are factually bringing a lot of noise in the ordinal theory and confusing many end users. Inscription number is the info displayed in the forefront of explorers, etc. Sequencing inscriptions with inscription numbers is useful for development purposes, but it’d be for the best if they could become “secondary”, since the sequence is flawed and broken.
Let’s go back to the root and tell a story around ordinal numbers instead of inscription numbers?

That being said, I agree on the thus not throwing off inscription numbers, this bug was discovered 3M inscriptions ago.
To build on top of what you’re saying, I think we should also ignore the content of these inscriptions (2 as of today), so we make sure that this flaw is not used in the future for creating inscriptions that could break downstream protocols built on inscriptions.
TL;DR - introducing some sort of standardized tombstone inscription.

@supertestnet
Copy link

supertestnet commented May 13, 2023

I think the thing to do is to recognize the inscription, thus not throwing off inscription numbers, but treat it as having no location, i.e. as having not been made on any particular sat, and not having a location, output, or offset on the /inscription page.

If I want to transfer it to someone am I just out of luck? It feels like the inscription should go into the address I specified as an output so I can send it to someone in return for a payment. It's probably a very valuable inscription at this point.

Please don't steal from me by making my inscription unsendable! Just put it where I specified as an output.

@casey
Copy link
Collaborator

casey commented May 13, 2023

Please don't steal from me by making my inscription unsendable! Just put it where I specified as an output.

Fixing a bug isn't stealing ;)

@lgalabru
Copy link
Author

You should try to transfer it today, I'm not convinced you'd be able to - that inscribed sat is included in a sat range that you don't own (the miner does).

@jotapea
Copy link

jotapea commented May 14, 2023

I'm sure @supertestnet knows lol. Inscriptions don't have any definite connection to a sat, it is only a convention. His transaction is one of the best proofs, an inscription without any possible sat to connect it to! 🤯

@supertestnet
Copy link

supertestnet commented May 15, 2023

Fixing a bug isn't stealing ;)

I think it sometimes is. The Ordinal Theory Handbook states: "The inscription content is contained within the input of a reveal transaction, and the inscription is made on the first sat of its first output." source My transaction demonstrates a flawed assumption within that theory: the first output does not necessarily have a first sat. You could fix this bug by modifying ordinal theory so that if there is no sat in the first output the inscription becomes unsendable. But in my opinion a better way to fix it is to modify ordinal theory so that if there is no sat the inscription is sendable, but is not attached to any sat at all. (Similar to how utxos with 0 value are spendable but do not transfer any sats.)

If you modify your explorer so that it shows my inscription as belonging to the address I tried to send it to (as seems fair to me), I might assign it to an ordinal by making a new transaction that sends it to an output that has a positive value. Or I might send it to another 0 value output where the address is controlled by someone who wants to buy my inscription. (That way it can retain its unicity and become the first inscription without an associated ordinal.)

Regardless of what I choose, I think it should be my choice, and in order to make that choice, I need the inscription. It sounds like you disagree with my opinion about how to fix this bug and you think the inscription should be unsendable. I hope you can see based on my reasoning above why that seems like theft to me. I wanted the inscription in the address I designated, and a bug in your software left it appearing as if someone else's address holds it instead. If you fix the bug by making my inscription totally unusable, rather than making it show up in the address where I wanted it to go, that seems unfair and I don't understand why you would do that.

If you compare my proposed solution with your proposed solution, I think the solution where the inscription goes where I tried to send it is more logical, and the solution where the inscription becomes unsendable is more spiteful. But perhaps I am missing something.

@GaloisField2718
Copy link

Block 788200 is including a curious transaction

Inputs:

2814d0a3c9e6dd5b88911a6280fc3899391f5c47072eb11593af2838160fad2f:0 : 0 sats

Outputs:

bc1q6zpf4gefu4ckuud3pjch563nm7x27u4r5j4ua7 : 0 sats

No satoshi at play in this transaction. Yet, ord validated the inscription (3492721) attached to the input https://ordinals.com/tx/c1e0db6368a43f5589352ed44aa1ff9af33410e4a9fd9be0f6ac42d9e4117151 which sounds like a bug.

Philosophically, the satoshi inscribed was transferred to the miner as a transaction fee, but was nevertheless inscribed by its previous owner.

Even if the miner own this UTXO, this is not how they should be inscribing it, per the protocol (they should be inscribing the coinbase transaction).

As a consequence, all the satoshis inscribed after inscription 3492720 are off by one.

https://ordinals.com/inscription/2814d0a3c9e6dd5b88911a6280fc3899391f5c47072eb11593af2838160fad2fi0 is not an inscription is it normal ?

@lgalabru
Copy link
Author

lgalabru commented May 18, 2023

Turns out, this bug is a bit more vicious than I thought.
We have 2 occurrences of this issue: c1e0db6368a43f5589352ed44aa1ff9af33410e4a9fd9be0f6ac42d9e4117151i0 and 99e70421ab229d1ccf356e594512da6486e2dd1abdf6c2cb5014875451ee8073i0.

If we look at block 788200:

  • transaction c1e0db6368a43f5589352ed44aa1ff9af33410e4a9fd9be0f6ac42d9e4117151 is in 16th position in the block, getting the inscription number 3,492,721
  • transaction c4d52213c04a7f9f87cb762bfb6b4cb50cf31c4b0cfdfbf709ba434b67f88584 is one of the last transaction in the block, and getting inscription number 3,492,720.

So let's say we have a Block B1, with, in order, the N transactions T1, T2, TN with T2 materializing the sat overflow issue.
The inscription sequence ends up being:
T1 -> 1
T2 -> N
T3 -> 2
T4 -> 3
...
TN -> N-1

@GaloisField2718
Copy link

I think the thing to do is to recognize the inscription, thus not throwing off inscription numbers, but treat it as having no location, i.e. as having not been made on any particular sat, and not having a location, output, or offset on the /inscription page.

Can we assume that ordinals protocol indexes utxo.
By the way it's counting utxo rather than sats. But as most of utxo contains at least 1 sat protocol is good most of the time.
No ?

@GaloisField2718
Copy link

Fixing a bug isn't stealing ;)

I think it sometimes is. The Ordinal Theory Handbook states: "The inscription content is contained within the input of a reveal transaction, and the inscription is made on the first sat of its first output." source My transaction demonstrates a flawed assumption within that theory: the first output does not necessarily have a first sat. You could fix this bug by modifying ordinal theory so that if there is no sat in the first output the inscription becomes unsendable. But in my opinion a better way to fix it is to modify ordinal theory so that if there is no sat the inscription is sendable, but is not attached to any sat at all. (Similar to how utxos with 0 value are spendable but do not transfer any sats.)

If you modify your explorer so that it shows my inscription as belonging to the address I tried to send it to (as seems fair to me), I might assign it to an ordinal by making a new transaction that sends it to an output that has a positive value. Or I might send it to another 0 value output where the address is controlled by someone who wants to buy my inscription. (That way it can retain its unicity and become the first inscription without an associated ordinal.)

Regardless of what I choose, I think it should be my choice, and in order to make that choice, I need the inscription. It sounds like you disagree with my opinion about how to fix this bug and you think the inscription should be unsendable.

Lol you're angry. Maybe it's not the way that they think this.

I hope you can see based on my reasoning above why that seems like theft to me. I wanted the inscription in the address I designated, and a bug in your software left it appearing as if someone else's address holds it instead. If you fix the bug by making my inscription totally unusable, rather than making it show up in the address where I wanted it to go, that seems unfair and I don't understand why you would do that.

Can you visualise this tx in the actual explorer ?

Can you see this inscriptions via
ˋord wallet inscriptions` ?

If you compare my proposed solution with your proposed solution, I think the solution where the inscription goes where I tried to send it is more logical, and the solution where the inscription becomes unsendable is more spiteful. But perhaps I am missing something.

In Bitcoin protocol we can spend this utxo ?
If yes with which command in ˋbitcoin-cli ˋ ?

@GaloisField2718
Copy link

Turns out, this bug is a bit more vicious than I thought.

We have 2 occurrences of this issue: c1e0db6368a43f5589352ed44aa1ff9af33410e4a9fd9be0f6ac42d9e4117151i0 and 99e70421ab229d1ccf356e594512da6486e2dd1abdf6c2cb5014875451ee8073i0.

If we look at block 788200:

  • transaction c1e0db6368a43f5589352ed44aa1ff9af33410e4a9fd9be0f6ac42d9e4117151 is in 16th position in the block, getting the inscription number 3,492,721

  • transaction c4d52213c04a7f9f87cb762bfb6b4cb50cf31c4b0cfdfbf709ba434b67f88584 is one of the last transaction in the block, and getting inscription number 3,492,720.

So let's say we have a Block B1, with, in order, the N transactions T1, T2, TN, - T2 materializing the sat overflow issue.

What is -T2 ?
It's not in you're following sequence.

The inscription sequence ends up being:

T1 -> 1

T2 -> N

T3 -> 2

T4 -> 3

...

TN -> N-1

@casey
Copy link
Collaborator

casey commented May 24, 2023

@supertestnet Sorry dude, but your arguments are pretty unconvincing 😂 This is pretty obviously a trivial bug with a simple fix, and the opposing argument is, not to be too harsh, just silly.

But in my opinion a better way to fix it is to modify ordinal theory so that if there is no sat the inscription is sendable, but is not attached to any sat at all. (Similar to how utxos with 0 value are spendable but do not transfer any sats.)

Making it possible for an inscriptions to be sendable, but not attached to any particular sat would be a massive change that entirely departs from the existing paradigm of ordinals and inscriptions.

If you modify your explorer so that it shows my inscription as belonging to the address I tried to send it to (as seems fair to me),

See above, but inscriptions do not and cannot belong to addresses. They belong to sats, which sit in outputs, which have addresses.

I might assign it to an ordinal by making a new transaction that sends it to an output that has a positive value.

This would require an entirely new transfer mechanism to be invented, as opposed to, oh idk, just fixing the bug 😂

Regardless of what I choose, I think it should be my choice, and in order to make that choice, I need the inscription.

So you'd prefer a change which gave you ownership of the inscription, ic ic 😂

I hope you can see based on my reasoning above why that seems like theft to me.

Not really. Aside from the above, fixing the bug wouldn't even be theft because you don't currently own the inscription. The inscription is currently owned by the person who owns the sat which was erroneously inscribed.

I wanted the inscription in the address I designated,

No you didn't, if you wanted an inscription you would have made a normal one 💁‍♀️

and a bug in your software left it appearing as if someone else's address holds it instead. If you fix the bug by making my inscription totally unusable, rather than making it show up in the address where I wanted it to go, that seems unfair and I don't understand why you would do that.

See above 😘

I do appreciate you finding this bug though!

See PR #2107 for the fix. Inscriptions made in transactions with no sats on the inputs are assigned to the 0…0:0 output, and thus have no location. Such inscriptions exist, so inscription numbers are not disturbed, but are unspendable, because the 0…0:0 output cannot be spent.

Ez bug, ez fix. Ordinal disrespecters should have found something that wasn't a nothing burger to sink their teeth into ;)

@casey
Copy link
Collaborator

casey commented May 24, 2023

Fixed by #2107.

@casey casey closed this as completed May 24, 2023
@lgalabru
Copy link
Author

@casey Could you please confirm / acknowledge that the related bug mentioned in this comment https://github.com/casey/ord/issues/2062#issuecomment-1553590928 is not fixed and will never be fixed?
This seems important for other implementations.

@casey
Copy link
Collaborator

casey commented May 24, 2023

@lgalabru Are you referring to the fact that inscriptions with zero-value inputs are processed out of order in the block in which they're found, so they receive inscription numbers as if they were the last transaction in the block? That's actually quite an annoying wrinkle. PR #2107 changes the order in which they're processed, so that they receive inscription numbers in the "correct" order. I'm not sure this is ideal, but the alternative would require alternative implementations to maintain the pretty odd behavior of assigning inscription numbers as if zero-value input inscriptions are found last in the block.

@lgalabru
Copy link
Author

I see. So around 5000 inscriptions (2 blocks) will see their inscription number changing with the next release?

@GaloisField2718
Copy link

@lgalabru Are you referring to the fact that inscriptions with zero-value inputs are processed out of order in the block in which they're found, so they receive inscription numbers as if they were the last transaction in the block? That's actually quite an annoying wrinkle. PR #2107 changes the order in which they're processed, so that they receive inscription numbers in the "correct" order. I'm not sure this is ideal, but the alternative would require alternative implementations to maintain the pretty odd behavior of assigning inscription numbers as if zero-value input inscriptions are found last in the block.

Is it possible that this error https://github.com/casey/ord/issues/2110#issue-1723875952 is coming from this update ?

@casey
Copy link
Collaborator

casey commented May 25, 2023

I see. So around 5000 inscriptions (2 blocks) will see their inscription number changing with the next release?

Not necessarily. I haven't decided what to do here! I implemented the fix in #2107 not realizing that it would change inscription numbers.

@lgalabru
Copy link
Author

Should we re-open this issue then? Or open another one to track the fact that anyone running the main branch is breaking consensus?

@dzyphr
Copy link

dzyphr commented May 30, 2023

I see. So around 5000 inscriptions (2 blocks) will see their inscription number changing with the next release?

Not necessarily. I haven't decided what to do here! I implemented the fix in #2107 not realizing that it would change inscription numbers.

Did you consider.... not spending time on scamming people?

@veryordinally
Copy link
Collaborator

@raphjaph fixed the renumbering issue and the fix is part of the 0.6.0 release.

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

No branches or pull requests