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

Add Spices for Bitcoin blocks, transactions and addresses #916

Merged
merged 11 commits into from
Jul 29, 2014

Conversation

biteasy
Copy link

@biteasy biteasy commented Jun 29, 2014

bitcoin_address
bitcoin_block
bitcoin_transaction

What does your instant answer do?
Spices for showing information about Bitcoin addresses, blocks and transactions based on the Biteasy.com Blockchain API.

What are some example queries that trigger this instant answer?

  • 000000000000000036a908fa64c2656f5e33e51050833c73866a81315ca75216
  • ebf42a5fb9b87e73c740fe45cd016d6b17beac3d764fc9952e54d5cda64bfca6
  • 1Biteasym3p5E4soZq8So6NjkjYugEnz2X

What is the data source for your instant answer? (Provide a link if possible)
https://www.biteasy.com

Which communities will this instant answer be especially useful for? (gamers, book lovers, etc)
Definitely useful for the bitcoin community

Which existing instant answers will this one supersede/overlap with?
There is no other spice for the transactions or the blocks. There is however a spice called BitcoinBalance but we believe that our version is more complete.

Is this instant answer connected to a DuckDuckHack instant answer idea?
I don't think so. I haven't seen anything in the ideas forum.

Are you having any problems? Do you need our help with anything?
Everything seems ok on our side. If there are proposed changes we will happily make them.

Checklist

Please place an 'X' where appropriate.

[X] Added metadata and attribution information
[X] Wrote test file and added to t/ directory
[X] Verified that instant answer adheres to design guidelines(https://github.com/duckduckgo/duckduckgo-documentation/blob/master/duckduckhack/styleguides/design_styleguide.md)
[] Tested cross-browser compatability

    Please let us know which browsers/devices you've tested on:
    - Windows 8
        [] Google Chrome   
        [] Firefox         
        [] Opera           
        [] IE 10           

    - Windows 7
        [] Google Chrome   
        [] Firefox         
        [] Opera           
        [] IE 8            
        [] IE 9            
        [] IE 10           

    - Windows XP
        [] IE 7            
        [] IE 8            

    - Mac OSX
        [X] Google Chrome   
        [X] Firefox         
        [X] Opera           
        [X] Safari          

    - iOS (iPhone)
        [] Safari Mobile   
        [] Google Chrome   
        [] Opera           

    - iOS (iPad)
        [] Safari Mobile   
        [] Google Chrome   
        [] Opera            

    - Android
        [] Firefox         
        [] Native Browser  
        [] Google Chrome   
        [] Opera

@jagtalon
Copy link
Member

Hey, @biteasy! Thanks for submitting! I'll get back to you on this. :)

@biteasy
Copy link
Author

biteasy commented Jun 30, 2014

Thanks @jagtalon , waiting for your feedback :)

call_type => 'include',
caller => 'DDG::Spice::BitcoinAddress',
proxy_cache_valid => "418 1d",
),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add tests for the example primary queries (for eaxmple 1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa) on all the the spices?

@mintsoft
Copy link
Collaborator

mintsoft commented Jul 2, 2014

@biteasy Thanks for the submission, I've added a few minor bits of feedback prior to @jagtalon finding a window to give it a more thorough once over :)

@biteasy
Copy link
Author

biteasy commented Jul 3, 2014

@mintsoft , thanks for the feedback. Everything will be done and pushed today.

Regarding the styling to be pushed to bitcoin_address.css, the reason we added it inline was because the css seemed to need a few seconds to load and the UI wasn't looking right until it did. Will the css load faster in production and we were only seeing this due to localhost run?

@mintsoft
Copy link
Collaborator

mintsoft commented Jul 3, 2014

@biteasy That's interesting behaviour, I've not actually seen that in the testing environments that I run. @jagtalon @moollaza have you experienced this?

@biteasy
Copy link
Author

biteasy commented Jul 3, 2014

@mintsoft, it might have been slow on the specific computer we've been running it. We will do the change and if the problem exists on your testing environments as well then we can reconsider.

@biteasy
Copy link
Author

biteasy commented Jul 3, 2014

@mintsoft, your requested changes are now committed.

@moollaza
Copy link
Member

moollaza commented Jul 4, 2014

@biteasy thanks a lot for making this PR, I had a quick chat about this IA and here's my general feedback:

Immediately after seeing your pics I noticed two things:

  1. You're using the auxiliary box for the what is really the most important part of the IA
  2. You're pushing static content into the detail area

There's no need for the static text explaining a block or a transaction. Instant answers generally don't explain what they're showing to you - the idea is that they should be informative to the kinds of people who will use them. For example our DNA Reverse Complement IA doesn't explain what any of that stuff is (I have no clue what a reverse complement is, but if I want to know I can also search DDG to find out) and for anyone who's going to use the IA often, seeing that explanation will likely become a little annoying.

With regards to using the Auxiliary box, I'm super glad to see someone using it (was it easy to use? I'm also curious why you chose to use it?) but as mentioned you've put the most important information into there which is off to the side and generally supposed to include supplementary, related or "auxiliary" information, while the detail area, which is front and center, should be where the result goes.

I suggest you switch to the Text or Info template group and use the record template (inside the content area) to display this data in a very similar fashion to the Aux box. You can even turn on rowHighlight to make it a little easier on the eyes. Please avoid the use of the base template group unless you absolutely need to use it for some very special markup (which in general we try to avoid).

Also, I tried scanning the QR code and was a little dissapointed to see it didn't do anything special other than show me the same hash that I alread searched. I think we can do without the QR Code, they're generally neat to see but not really usable, especially in this case -- I don't see a use case where you'd need to pull out your phone to scan a QR code on your computer screen...

Regarding the CSS as mentioned please use stylesheets rather that inline CSS. If you could provide more details about the issue you were having with DuckPAN I'd be glad to look into it and see if something needs to be fixed. Ideally though, the use of templates should alleviate the need to use any CSS.

Please let me know what you think about all this!

@biteasy
Copy link
Author

biteasy commented Jul 5, 2014

@moollaza, first of all thanks for the feedback. Below are my comments:

  1. You're using the auxiliary box for the what is really the most important part of the IA
  2. You're pushing static content into the detail area

We modeled the spices based on this IA for the U.S dollar. However I understand that the U.S dollar IA is generic and refers to the currency while we are referring to specific blocks addresses and transactions so the static text is probably not needed.

With regards to using the Auxiliary box, I'm super glad to see someone using it (was it easy to use?
I'm also curious why you chose to use it?) but as mentioned you've put the most important
information into there which is off to the side and generally supposed to include supplementary,
related or "auxiliary" information, while the detail area, which is front and center, should be where the > result goes.

The auxiliary box was pretty easy to use especially since we could find this spice using it. We chose the auxiliary box for 2 reasons basically. First, because of the U.S dollar IA which we assumed was similar to what we were displaying (information about a currency). Second, some of the spices especially the one about blocks has a lot of rows to display. If we tried to cram all that info in the main content that would make the height quite big and hide all the organic results.

I suggest you switch to the Text or Info template group and use the record template (inside the
content area) to display this data in a very similar fashion to the Aux box. You can even turn on
rowHighlight to make it a little easier on the eyes. Please avoid the use of the base template group
unless you absolutely need to use it for some very special markup (which in general we try to avoid).

We can definitely use something like the record template but this will make the height of the IA really big in some cases. Do you suggest that we display some info in the main content and the remaining (or all) in the auxiliary box?

Also, I tried scanning the QR code and was a little disappointed to see it didn't do anything special
other than show me the same hash that I already searched. I think we can do without the QR Code,
they're generally neat to see but not really usable, especially in this case -- I don't see a use case
where you'd need to pull out your phone to scan a QR code on your computer screen…

Displaying the QR code for an address is pretty standard for all blockchain explorers. Basically the QR code is an easy way to transfer the address to a mobile phone which would be a nightmare to type manually. If you scan the QR code with a wallet application on Android it will immediately create a new transaction to this address. All you have to do is type in the amount and tap send to perform the transaction. You can try this using this Android application. Wallets are starting to pop up on iOS too after the recent Apple policy changes so you can do the same on iOS.

Regarding the CSS as mentioned please use stylesheets rather that inline CSS. If you could provide
more details about the issue you were having with DuckPAN I'd be glad to look into it and see if
something needs to be fixed. Ideally though, the use of templates should alleviate the need to use
any CSS.

We already moved the styling to css with our latest push. It was most probably an issue of our development machine. If we encounter this issue again we will make a proper report.

@moollaza
Copy link
Member

moollaza commented Jul 7, 2014

@biteasy thanks for helpful response, now I have a better understanding about your approach in developing this IA.

With regards to using the record template, I agree that height would become and issue. I think the Address IA will be fine with the record template because there's only a few rows and the Transaction IA might also be okay. The Block IA would likely be too tall though, if we put all the info into a record template.

My question is if all that data is really important and needs to be shown? Typically we try to make the IA's pretty simple and only show the most relevant info. The "More at" link is provided so users can follow through to more information if they need it.

I think the best approach would be to decide what data is most important and put that it into the record template. Anything else can be put into the aux box, or simply left out.

I think you should try out the record template so we can see how it looks. Once we have something to play around with our designer Chris can also give some feedback and we'll go from there.

@biteasy
Copy link
Author

biteasy commented Jul 8, 2014

@moollaza, we made the changes as suggested as shown in the screenshots below. We personally think that the original design looks better (specially the address with the QR code) but it's your call so please let us know how should we proceed. Please note that we haven't committed this change yet to our repository.

screen shot 2014-07-09 at 12 42 20 am
screen shot 2014-07-09 at 12 42 36 am
screen shot 2014-07-09 at 12 45 03 am

@moollaza
Copy link
Member

moollaza commented Jul 9, 2014

@biteasy thanks a lot for implementing this, I agree that it is a little plain, we likely need to add a little more base styling to the record template. For now I suggest you mimic the CSS for the LeakDB spice and turn or rowHighlight, the result will look much nicer.

LeakDB Example:
image
Here's the LeakDB Code: https://github.com/duckduckgo/zeroclickinfo-spice/blob/master/share/spice/leak_db/leak_db.css

Regarding the QR, sorry I meant to say that I think it can stay given the reasons you pointed out. For the Bitcoin Address Spice I think you should use the Info template as it will allow you to provide an image, title and for the content you can have it use the record template.

@biteasy
Copy link
Author

biteasy commented Jul 11, 2014

@moollaza We have implemented the changes as suggested and indeed they look much better. Bellow you can see some screenshots for the Spices.

screen shot 2014-07-11 at 9 46 45 pm
screen shot 2014-07-11 at 9 45 51 pm
screen shot 2014-07-11 at 9 45 23 pm

We have also pushed the changes. In order to test on your side we will have to provide you with an API key.

@moollaza
Copy link
Member

@biteasy wow this looks awesome! I'm quite happy with how this is shaping up. @chrismorast any thoughts?

@biteasy please email me an API key: moollaza@duckduckgo.com

@biteasy
Copy link
Author

biteasy commented Jul 14, 2014

@moollaza We just removed all the js infobox styling from all spices since its not used any more. API key has been sent to your email as well.

@moollaza
Copy link
Member

Sounds good, thanks

moollaza added a commit that referenced this pull request Jul 29, 2014
Add Spices for Bitcoin blocks, transactions and addresses
@moollaza moollaza merged commit 2030ef5 into duckduckgo:master Jul 29, 2014
@biteasy
Copy link
Author

biteasy commented Jul 30, 2014

@moollaza , awesome! Does this means we will see the it on the live site soon?

@moollaza
Copy link
Member

@biteasy it has to go through an internal release process and deployment and then it will be live. I'll be sure to notify you guys here when that happens (usually within a couple days).

@biteasy
Copy link
Author

biteasy commented Jul 30, 2014

@moollaza , great! Looking forward to it. Let us know if you need anything else on our side.

@jagtalon
Copy link
Member

jagtalon commented Aug 6, 2014

Excited!

@luke-jr
Copy link

luke-jr commented Aug 14, 2014

This is broken. Bitcoin addresses do not have balances, and do not send bitcoins. Addresses only receive. It would be nice if DuckDuckGo would not encourage confusion about Bitcoin. :(

@thereisnobalance
Copy link

Bitcoin addresses do not in any way have a "balance", and the misconception of them having one is causing a lot of issues for us at the moment. It is counter-productive in teaching people about Bitcoin, and it subtly encourages people to re-use receiving addresses.

  • The number being accounted for in this case is really "spendable outputs", that is a total of the value of spendable fractions of Bitcoin available to be spent by somebody who can prove they own a particular pubkeyhash. The distinction seems insignificant but makes a great deal of difference when it comes to describing the technical aspects of our data storage and network rules. Presenting it in this way causes users to be mislead into thinking that Bitcoin is balance-based, when really it resembles nothing that the clients do under the hood. When it comes to explaining change addresses this is often a source of major confusion.
  • Treating Bitcoin receiving addresses as anything other than single-use identifiers for payments encourages people to reuse keys for multiple payments, even have them as permanent identities. Having them displayed like this in other services have frequently lead users to believe that they are analogous to looking at the value of a bank account, meaning they use them for payments again and again. Re-using a receiving address more than once makes a users behavior extremely easy to track, removing what little privacy there is in the currency.

I don't believe this is appropriate information to be showing to users.
I strongly urge that address "balances" be removed.

@alolis
Copy link

alolis commented Aug 14, 2014

@luke-jr , non technical people do not know and do not care about the technical aspect of bitcoin, they just need to read words they can grasp. Besides, the balance does exist, it's just the sum of all unspent outputs for a specific address. Just because a word which is more easily understood by everybody is used but not specified in the bitcoin spec, does not make it wrong.

@thereisnobalance, nice newly created account. Maybe next time you will have more credibility if you use your real name.

All block explorers use the word "balance" and "received" which is just an easier word for everybody to understand. Users do not care what's going under the hood, stop confusing those two things and maybe bitcoin will reach even bigger audience.

@luke-jr
Copy link

luke-jr commented Aug 14, 2014

@alolis Re "non technical people do not know and do not care about the technical aspect of bitcoin", that is exactly my point. The balance does not exist. Addresses don't have unspent outputs, that is what is the technical details. The number being displayed as a "balance" here is not significant in any way at all to anyone (except maybe forensics detectives).

Bugs on one website should not translate into bugs being introduced in others.

@thereisnobalance
Copy link

@alolis I don't use my real name for anything as a matter of choice, getting vitriol from random people on Github for it backs up that decision. You can have a go at me on freenode#bitcoin if you truly believe my response to be a sybil, however this has absolutely no relevance to the issue at hand. I have long been using throwaway github accounts to raise these sorts of issues .

ALL block explorers use the word "balance" and "received" which is just an easier word for everybody to grasp.

Blockchain.info also shows some addresses as having a negative balance; just because another party is doing something stupid doesn't mean that action is correct.

It's largely concluded by the technical members of the community (of which I am one) that block explorers showing this information is harmful, it's in my interests to not have a search engine making the same mistakes as them too. I have spent an unbelievable amount of time un-teaching the misinformation which block explorers have taught, so you can understand my reservation of having this on DDG as well.

@traboukos
Copy link
Contributor

It's largely concluded by the technical members of the community (of which I am one)

@thereisnobalance If you are going to make an argument on authority shouldn't you at least reveal your name ? Your points definitely do not seem absurd though so what would you suggest as a remedy ? Balance row totally removed ? renamed ?

@luke-jr
Copy link

luke-jr commented Aug 14, 2014

@traboukos I have no issue revealing my name and credentials, at least... I am a Bitcoin developer who has been studying Bitcoin at a technical level since 2011 Jan, and a prolific contributor to many prominent Bitcoin software and education projects. A bitcoin address should only show (total) "Received". "Current Balance" and "Total Sent" are wrong, and the numbers there have no relevance to the address.

@traboukos
Copy link
Contributor

@luke-jr I know who you are and my comment was not addressed to you. What is your opinion on the following ?

"balance" here is not significant in any way at all to anyone (except maybe forensics detectives)

Isn't that what a block explorer is ? A tool to perform forensics and analytics on the blockchain ? I understand when this is concerning a wallet. That definitely has the potential to confuse the user and cause him to reuse addresses. A blockexplorer on the other hand and by extension a search engine like DDG is not a wallet. Their job is to gather, search and analyse data.

@luke-jr
Copy link

luke-jr commented Aug 14, 2014

A block explorer, if intended to be used for forensics, might see some reason to talk about UTXOs and internals like that - but in that case, it should be using the proper terminology, even if that's not immediately obvious to novices, so that people don't get confused over what addresses are or how Bitcoin works for the end user.

Correct me if I'm wrong, but I have the impression DDG is meant for end users, not forensics/detectives. If it is being given a bitcoin address, it should display information that is relevant to that address itself (which does not include "total value of unspent transaction outputs redeemable with the keypair AABBCCDD" keeping in mind that the keypair is not the same as the address implemented using it). If someone wants to dive into the technical and/or forensics side of things, isn't that what the Biteasy link is for?

@traboukos
Copy link
Contributor

@luke-jr I am still a bit mixed about this but we can issue a new pull request and fix this if convinced.

@jagtalon @moollaza I know you might not might not be completely up to date about bitcoin technicalities but what is your take on this ?

@alolis
Copy link

alolis commented Aug 14, 2014

I think @traboukos covered what I wanted to say as well.

Blockchain.info also shows some addresses as having a negative balance; just because another
party is doing something stupid doesn't mean that action is correct.

That's probably a bug on their part and not intended but I can not speak for Blockchain.info. You might want to clear that up with them.

@thereisnobalance
Copy link

That's probably a bug on their part and not intended but I can not speak for Blockchain.info. You might want to clear that up with them.

@alolis They're very much aware. The point was that you can't use "everybody else is doing it" as a justification for this sort of issue.

@thereisnobalance
Copy link

A tool to perform forensics and analytics on the blockchain ? I understand when this is concerning a wallet. That definitely has the potential to confuse the user and cause him to reuse addresses. A blockexplorer on the other hand and by extension a search engine like DDG is not a wallet. Their job is to gather, search and analyse data.

@traboukos Yes, it's an unfortunate issue that people have begun to use block explorers as a everyday tools. Generally the users aren't aware of what they are looking at (which makes sense, it's a technical thing to be viewing) and make unwise judgements based on this information. Very often the conclusions drawn by people using this sort of data are fairly incoherent as a result.

@zekiel
Copy link
Member

zekiel commented Aug 15, 2014

We'd like to arrive at a decision for this change and, to the points raised above, I want to mention that our aim for instant answers is to make them as canonical/correct as possible. It sounds like the de facto term for "balance" is "spendable output" among the bitcoin community. Although it's not a mainstream reference, it does represent the underlying data more accurately (or so it's suggested).

Is everyone on this thread ok with that change? I.e. are there any strong reasons not to call balance, "spendable output"?

@luke-jr
Copy link

luke-jr commented Aug 16, 2014

The spendable outputs refers to the scriptPubKey, not to the address. These two are technically related, but not in a meaningful sense to end users. When outputs get spent (removing them from the key's spendable outputs), that action is entirely unrelated to the address. Displaying it as information for the address is therefore not correct at all.

@traboukos
Copy link
Contributor

@zakiel, from what I understand @luke-jr does not have a problem with the naming but he does not want that information to be shown at all. That is however an opinion which is not shared by everybody and definitely not the consensus of the Bitcoin community. Other developers that we talked with agree that addresses clearly have balances which is a very simple and intuitive concept with a crisp definition for Bitcoin that can be easily calculated. From an end users perspective, if a user searches for an address I think he expects to see the balance and any other incoming/outgoing traffic that has been going on.

If however we are convinced that this creates confusion and there is substantial data to back it up or more voices from the Bitcoin core dev community are heard we have no problem removing the balance feature.

@luke-jr
Copy link

luke-jr commented Aug 16, 2014

@traboukos You're demonstrating that this "feature" even confuses you. Addresses do not have outgoing traffic, only incoming.

@mikehearn
Copy link

The right term to use is clearly balance. "Spendable output" doesn't mean anything to a regular user, balance does. Every block explorer talks about address balances. It's a very simple and easy to calculate concept that maps naturally to how people think. This mental model breaks only for use cases that are today quite unusual, e.g. multi-signature wallets, and even then via P2SH we still have an address-like thing which can be said to have a balance. So it doesn't break much.

BTW I'm a Bitcoin developer and author of a library that has been used to create over 2 million wallets. There is no consensus in the community around @luke-jr's arguments, much though he enjoys making them.

@traboukos I suggest just rolling with the original code and lingo. There are more important things in life to argue about than this.

@traboukos
Copy link
Contributor

Thanks @mikehearn for chiming in. Your feedback is really appreciated.

@luke-jr
Copy link

luke-jr commented Aug 16, 2014

@mikehearn You're leaving me no choice but to point out that wallets based on your software are all broken/buggy when it comes to key and address management. You're part of the problem in this regard.

@mikehearn
Copy link

Yes, I know that you routinely label any design tradeoff you disagree with as broken and/or buggy. Unfortunately all that achieves is making it harder to know when you're reporting actual bugs as opposed to things you just don't like. Please Luke, words are valuable, let's not waste them.

Anyway, even if address reuse goes down (and it will), addresses will still have a meaningful notion of balance. It might not be particularly useful but it will still exist and be well defined.

@zekiel
Copy link
Member

zekiel commented Aug 18, 2014

Thanks, all.

If there's a place to point wrt canonical naming conventions and whether this data should be shown together, we can certainly revisit. Hopefully, this is constantly updated for improvement.
For now, though, I think this is okay. Haven't seen any other feedback on it and it's pretty tomato tomahto right now :)

@alolis
Copy link

alolis commented Aug 19, 2014

@zekiel , if any changes are required in the future, we are around.

@zekiel
Copy link
Member

zekiel commented Aug 19, 2014

@alolis very much appreciated! Thanks!

Also, many thanks to everyone for their feedback on this thread. We'd love to hear more constructive feedback like this for all of our instant answers so don't be shy!

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

Successfully merging this pull request may close these issues.

None yet