blockcypher / explorer

Block explorer showcasing the BlockCypher APIs.
https://live.blockcypher.com
Apache License 2.0
1.07k stars 717 forks source link

Add detected data protocol to transaction view #150

Closed acityinohio closed 8 years ago

acityinohio commented 8 years ago

Just added the data_protocol field to the transaction hash API, would be awesome to expose this data on the transaction view of the explorer.

Here's an example transaction with data_protocol detected (factom): https://api.blockcypher.com/v1/btc/main/txs/7166f3aa854ebee1b1df49b3d67cf1dccd601c8f8cadac503be6f92dec3c77c5

And the full description of the new field in the documentation: http://dev.blockcypher.com/#tx

mflaxman commented 8 years ago

Very cool!

How does this relate to the data_hex that's returned? Basically, my thinking is that data_protocol is a transaction level entry so that should be shown for the whole transaction, while data_hex is an output level entry so that should be shown on just that output? That's a bit strange though to have the display in different places. Is it possible for a tx with a data_protocol to have multiple outputs with a data_hex?

What do you recommend doing here?

acityinohio commented 8 years ago

Thanks @mflaxman!

You're right on the money on where to put both the data_hex (at the TX Output level) and data_protocol (at the TX level). Agreed it's a little strange, but @matthieu and I chatted about it before implementing, and from an API perspective it definitely makes sense to put the data_protocol on the TX level instead of the TX Output level. This is becuase non-null_data TX Outputs (which lack data_hex) within the same transaction as the null_data TX Output occasionally have abstract, protocol-level meaning...depending on the protocol.

Also, it's very unlikely that a TX will have more than one TX Output with a data_hex . The current isStandard default for most miners will refuse to include transactions with more than one null_data TX Output...there are occasional non-isStandard transactions that get included in the blockchain, but it's also quite unlikely that any of them would have any data_protocol detected. Consequently, I think we're safe separating them, even if it seems a little strange. What do you think?

mflaxman commented 8 years ago

Hey @acityinohio, can you take a look at what I just pushed?

The feature was very simple, but the UI is a lot trickier.

Other ideas: 1) Bury the alert in the advanced details tab. This would make the page less cluttery but I think it hides important info. I could put the notice there and have a "more" button that opens the advanced details tab and includes the hex/string there. 2) It would be good to link out somewhere to explain what this is. Right now I'm linking to http://dev.blockcypher.com/#tx because that's where data_protocol is explained (not much though). I'm not linking out to data_hex/string here, which I easily could do but I think is more confusing. 3) Are there transactions that have an output with a data_hex/string but not a data_protocol? I don't think so but wasn't 100% sure. Either way, I think the docs should be explicit about any connection between the two.

I'm open to anything, just tell me what you like and/or feel free to submit a PR.

Also note that the UI here is related to #142 that I'd like to do soon.

mflaxman commented 8 years ago

Update: here's data_hex/string without data_protocol:

https://live.blockcypher.com/btc/tx/b43c517dc2f10317651aa6047446447da63611abf109e1b44d829cfd73bbbd9d/

Reopening because I have to fix this, but assigning to you @acityinohio for some feedback first on the best approach. Thanks!

mflaxman commented 8 years ago

Also, @acityinohio or @matthieu can you confirm that the lack of data_protocol attribute on the previous transaction (https://live.blockcypher.com/btc/tx/b43c517dc2f10317651aa6047446447da63611abf109e1b44d829cfd73bbbd9d/) is as intended? It feels like a bug to me.

acityinohio commented 8 years ago

Hi @mflaxman, yup! To answer your last question/number 3 first, last transaction won't have a data_protocol field; to detect data_protocol we use the first two to three bytes (the "magic bytes") in the OP_RETURN, and check if matches known protocols (Open Assets, Factom, etc). If it doesn't match, we don't return that field. (so there can be many OP_RETURN-containing transactions without data-protocol)

Regarding questions 1/2, I'm actually not seeing any update on the UI...not seeing data_hex/string anywhere on the block explorer. Here's a screenshot of my view:

screenshot 2015-10-13 at 11 15 55 am

mflaxman commented 8 years ago

Hey @acityinohio , that transaction doesn't have one because I'm only checking for data_protocol on display.

Try the one from earlier in the thread: https://live.blockcypher.com/btc/tx/7166f3aa854ebee1b1df49b3d67cf1dccd601c8f8cadac503be6f92dec3c77c5/

Or the one from #142: https://live.blockcypher.com/btc/tx/d82f637798d24097a1f7412414b0f0be6ddb7d756a0c7daf4b1814f73b2c4717/

The tricky part that I'm coming back to is that the action is on the Tx output but (I think) we want to show it on the TX as a whole.

What do you think of how I've done it (see qs above)? Also, what about for transactions without data_protocol that just have data_hex/string? Should I show them at the whole TX level or on the output. Keep in mind #115, which perhaps in this context I should close/delete.

there can be many OP_RETURN-containing transactions without data-protocol

Another thing to keep in mind is that I'm only grabbing some of the inputs/outputs of each txn (I believe 100 but am not 100% sure and would have to check). Theoretically, if there was null-data embedded in an output > than that number, I'd have no way of knowing when I render the page. That's why I like that data_protocol is on the whole Tx. That may be a non-existent problem, but I don't like the inconsistency. What I'm saying is that I think embedded null data of any type should be a flag for the whole tx (like data_protocol, which it could be subsumed into).

acityinohio commented 8 years ago

Ahh gotcha @mflaxman, sorry, I wasn't following before/didn't grok the full context of the thread.

I like what you've done for the design for data_protocol-detected transactions, but as you suggest, it will be problematic to show non/unknown-protocol detected OP_RETURN-containing transactions. As you suggest, maybe it would make sense to make the data_protocol a universal flag for anything containing a null-data output...perhaps adding an unknown data_protocol as a catchall for null-data? Would be a non-breaking change on the API and very simple (e.g., even I can update it, heh).

@matthieu does that make sense to you? If so I can do a PR on the API and let you know when it's done @mflaxman .

mflaxman commented 8 years ago

:+1: !

Also, I don't have much real-world experience with null data, so if I'm not understanding the use-cases or just generally off feel free to do something else.

mflaxman commented 8 years ago

@acityinohio @matthieu nudge!

Embedding data in the blockchain is live: https://live.blockcypher.com/embed-data/

But I'm not linking to it anywhere because once embedded the explorer isn't showing anything without a data_protocol attribute.

acityinohio commented 8 years ago

Nice @mflaxman! Apologies for delay, had a bit of a backlog but will submit a PR for the data_protocol change soon.

matthieu commented 8 years ago

I'd add somewhere there's a 40 bytes limit (so 80 hex chars or 40 ascii chars) no?

acityinohio commented 8 years ago

An error does show up if you attempt to submit something longer: "Data too long, the maximum length allowed by the bitcoin protocol is 40 characters." But agreed, even nicer glitz would include a character counter depending on type (e.g., updated interactively as you type: 0/80, 1/80, 2/80, etc. for hex, 0/40, 1/40, 2/40, etc. for ASCII)

mflaxman commented 8 years ago

Awesome, thanks guys.

This github issue is closed, but I appreciate your feedback on the embedding UI. I've created a new issue here: https://github.com/blockcypher/explorer/issues/153