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

Add detected data protocol to transaction view #150

Closed
acityinohio opened this issue Sep 28, 2015 · 14 comments
Closed

Add detected data protocol to transaction view #150

acityinohio opened this issue Sep 28, 2015 · 14 comments
Assignees
Milestone

Comments

@acityinohio
Copy link
Contributor

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

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?

@mflaxman mflaxman added this to the More Better milestone Sep 28, 2015
@acityinohio
Copy link
Contributor Author

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

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

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 mflaxman reopened this Oct 13, 2015
@mflaxman mflaxman assigned acityinohio and unassigned mflaxman Oct 13, 2015
@mflaxman
Copy link

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
Copy link
Contributor Author

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

@acityinohio acityinohio assigned mflaxman and unassigned acityinohio Oct 13, 2015
@mflaxman
Copy link

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

@mflaxman mflaxman assigned acityinohio and unassigned mflaxman Oct 13, 2015
@acityinohio
Copy link
Contributor Author

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 .

@acityinohio acityinohio assigned matthieu and unassigned acityinohio Oct 13, 2015
@mflaxman
Copy link

👍 !

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

@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
Copy link
Contributor Author

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

@matthieu
Copy link
Contributor

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

@acityinohio
Copy link
Contributor Author

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

Awesome, thanks guys.

This github issue is closed, but I appreciate your feedback on the embedding UI. I've created a new issue here:
#153

mflaxman pushed a commit that referenced this issue Oct 17, 2015
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

3 participants