Wallets: transparency score/source code requirements

[schildbach]

I think we should clarify and tighten the transparency score/source code requirements. Here is my proposal:

1. Sources shall be readable and non-obsfuscated. None of the currently published sources are obsfuscated, but still it should be noted.
2. Sources shall be complete for the whole client app, plus server/services needed for receiving and sending payments. Good examples: Electrum is open source for its client and server. Bitcoin Core is open source and needs just other instances of itself for payment. Bad example: Mycelium, client is open source but service is not. Since the client is more or less just a frontend on the service and you cannot send or receive payments without it, essential parts are closed source.
3. Source shall be under version control. Reason: Without version control, sourceballs need to be reviewed all over again each time something is changed. With version control, only changes need to reviewed.
4. Source control must not be "restarted" unless the app is entirely new. Reason as above, we want to be able to review based on the changes. If a source tree is (re)started, it should get the "new app" penalty in any case. Recurring rebasing on another source tree doesn't count as restart, as long as the base isn't restarted. Bad example: blockchain.info restarted their source repository several times in the past weeks.
5. For each binary release, there must be a distinct source release at the same time or earlier. The source release shall be marked using a tag named like the binary version, or a commit with the binary version in its comment. Reason: If the source does not match the binary, we are reviewing the wrong thing.

What do you think?

[harding]

@schildbach if you don't mind, can you edit your post to use an ennumerated list? That might make discussion easier/clearer.

@saivann right now I think the only place where we store requirements is in the descriptions themselves. If we adopt @schildbach's recommendations (or something derived from them), we probably need a place to store them. Should we start a README.md in some directory?

[schildbach]

@harding Good points. Enumerated.

[schildbach]

Added one sentence about rebasing to 4.

[harding]

I think there are two different kinds of points here: 1, 2, & 3 are states but 4 is an event and 5 can be a state or an event.

It's easy to say, "if your code is in [this state], we'll give you a bad transparency score. If you fix the problem, we'll upgrade your score." However, I'm not sure what we should do about events. Do we penalize them by saying, "if you had [this event] in the last [month], you get a bad transparency score"? For example, for how long would we penalize BC.i because they restarted?

For 5, I think we'd have to decide whether to treat an untagged release as a state which can be immediately rectified or as an event which caries a penalty.

Switching focus: I like the idea behind 2, but I'm not sure we should focus on server software---after all, the person running the service can run any code they want. Mycellium already gets flagged as being Centralized, so I'm not sure we should penalize them for not releasing their server code as open source---reading that code won't make us more secure.

I wonder if we should limit our scrutiny to only the benefits declared by that particular wallet. For example, the BC.i app gets a good score for giving users "hosted control over their bitcoins". So, in my mind, that means the open source app should include the code that creates, stores, and uses the private keys without leaking data to BC.i. If the tx sending/receiving code is proprietary, that's ok because we have a Centralized warning.

[schildbach]

I see your point about not specifying the amount of penalty. I deliberately left that out. We're already penalizing one (or two?) new apps and one "restarted" app and need to answer the question anyway, independant of this proposal. If we know the answer, we can add it here.

Untagged releases should not warrant a penalty but only change the "state". Penalty isn't meant as a punishment (it would be extremely unfair to new apps). The reason for the penalty is it takes time for everyone to get to know the app and for enough people to review the source. Forgetting to tag just delays a review, but it doesn't make you review everything again.

The client side of apps always need to be fully open source, because as soon as you have one line of closed source you can hide the backdoor there. This is why I applied the notion of "partially open source" only on server software. You're right, we cannot know for sure if the service runs the code it claims to. Still it adds to transparency.

[harding]

Are we currently penalizing any apps? In the case of BC.i, I think we're just waiting for a response to the questions raised by you and Saïvann. I'm not sure why the Hive iOS pull is stalled; I'm guessing maybe nobody has taken time to review their repository. Is there a third app?

I think it's reasonable for the 'penalty' to be that the wallet provider must explain why the repository restart occurred, assure us that it won't happen again except under extreme circumstances, and for us to review the new repository with the understanding that it may take awhile as we all have better things to do.

I think it's reasonable to have a tag for partially open source to mean that the app's operation is verifiable through open code up to the claimed security and privacy levels. Was that in your proposal? I can't seem to find it, or maybe I'm misinterpreting what you mean by "I applied the notion of 'partially open source' only on server software."

[saivann]

Regarding 2 (open-source servers), I tend to agree with @harding, this is why I've created the "Decentralization" score. If we assume the server is only serving block chain data, the real worry here I think is who is controling the server used by the client, not if the server is open-source. Giving a bad score to open-source clients would not really be helpful for the final user IMO.

Otherwise I agree with 1, 3, 4, 5 so long as we're dealing with states only. e.g. 5 (release tags), we could penalize an app until they set the appropriate tag or commit, 4 (source control restarted), we could set the "new app" score for a few months much like new apps.

This leads me to asking for how long should we apply the "New app" score to new apps? How about 6 months from the release (or source control restarted) date?

@harding Currently, blockchain.info Android app is penalized because it has been restarted and was outdated since a while, and I've suggested applying the "New app" score to Hive Web / iOS as well as other new recent codebase, but this was only a suggestion at this point.

[harding]

@saivann that all sounds reasonable to me. Maybe we should put these criteria on the Bitcoin.org wiki?

[schildbach]

I'm ok with just defining 1, 3, 4 and 5 and yes @saivann handling states sounds good to me.

What about putting it into the either code comments on the choose-your-wallet page source? Or at at somewhere else into the bitcoin.org repo? I would not use the Bitcoin Wiki any more.

6 months sounds good as well.

[harding]

@schildbach I don't have any objection to putting it in code comments as long as they don't get rendered into the actual page---I'd hate to serve an extra ~500 bytes for each page load.

Also, the wiki I linked to is the GitHub wiki for Bitcoin.org where we keep the dev doc style guide and todo list. It's a git repository itself with markdown pages: Here's the URL: https://github.com/bitcoin/bitcoin.org/wiki

[saivann]

@schildbach @harding I'm neutral on either keeping this somewhere or just refering to this issue afterwhile. But perhaps we should be careful to keep things simple (e.g. avoid going further and explaining each score as they're mostly self-explanatory). IMO, these criterias are very reasonable so should someone report any of these issues with some codebase, I doubt we'll disagree.

As far as I'm concerned, I'm fine with just this issue, the Wiki or the README. An HTML comment on the page isn't something I expect we'll be refering to so I'm less enthusiastic with this option.

[schildbach]

I wasn't thinking of an HTML comment, but a code comment on the piece of code the transparency scores are configured.

[harding]

I hybridized @saivann's suggestion and created a github wiki page for notable issues so we can quickly re-find this issue even after it's closed.

https://github.com/bitcoin/bitcoin.org/wiki/Notable-Issues-&-Pulls

[saivann]

@schildbach @harding Maybe you can give me a day or two so I can try creating a short summary in the README which would look as follow? (it would at the same time be useful for me when new wallets are submitted)

---

Wallets

Screenshot: Screenshots must go in ... and be 250px X 350px ... optimized with ...

Icon: ...

Description: ...

Entry and properties: ...

Score

Control: In order to get a good score, the app must...

Decentralization: In order to get a good score, the app must...

Transparency: In order to get a good score, the app must...

Environment: In order to get a good score, the app must...

Privacy: In order to get a good score, the app must...

[harding]

@saivann I think that would be excellent!

[saivann]

@harding @schildbach I just opened pull reqs #529 and #530 .

[saivann]

This issue should now be fixed.