Forum rules


Warning !
Avoid using binary softwares from untrusted users.
Prefer compiling it yourself and verify sources.



Reply to topic  [ 12 posts ]  Go to page Previous  1, 2
Namecoin Local PGP Keyserver Plugin 
Author Message

Posts: 1919
os: linux
Reply with quote
Post Re: Namecoin Local PGP Keyserver Plugin
Some preliminary review:

https://github.com/phelix/npkh/blob/8c8 ... er.py#L208

It's not immediately obvious to me what the purpose of idFprs is. Is this a cache of some kind? Maybe add some comments explaining this.

https://github.com/phelix/npkh/blob/8c8 ... er.py#L212

It's not immediately obvious what the threat model of proxy_to_standard_pks . It looks to me like this function assumes that the caller is responsible for verifying the authenticity of the data returned by the remote keyserver. Is that correct? Maybe add some comments about this.

https://github.com/phelix/npkh/blob/8c8 ... er.py#L231

It's not immediately obvious to me what searchFpr will contain. It looks like this might be a caching mechanism but I'm not certain. Adding some comments about this would be helpful.

https://github.com/phelix/npkh/blob/8c8 ... thproxy.py

It looks like you're distributing a copy of https://github.com/jgarzik/python-bitcoinrpc . Is that correct? I'd be more comfortable if there were instead a dependency on the original version, so that users don't have to audit whether your version is identical to Jeff Garzik's version.

I'll post some additional review later.

Cheers!

_________________
Jeremy Rand, Lead Namecoin Application Engineer
NameID: id/jeremy
DyName: Dynamic DNS update client for .bit domains.

Donations: BTC 1EcUWRa9H6ZuWPkF3BDj6k4k1vCgv41ab8 ; NMC NFqbaS7ReiQ9MBmsowwcDSmp4iDznjmEh5


Sat Sep 16, 2017 5:32 am
Profile

Posts: 1919
os: linux
Reply with quote
Post Re: Namecoin Local PGP Keyserver Plugin
More review (based on what GitHub says is Git commit 8c86d534ec0c40403ceb6bef66990f43e957d5c6)

https://github.com/phelix/npkh/blob/mas ... dler.py#L7

I'm fine with this legacy workaround, since Bottle 0.13 isn't in Debian Stretch right now.

Generally speaking, it is difficult to follow the intended flow of the code for common use cases (e.g. looking up an id/ identity). It would probably be helpful to add a comment block at the top of the file that summarizes the intended flow.

The feedback I've given so far is mostly related to auditability. I'm probably not going to try to audit the code further until the requested auditability requests are addressed. Once they are addressed, I will continue reviewing.

Cheers!

_________________
Jeremy Rand, Lead Namecoin Application Engineer
NameID: id/jeremy
DyName: Dynamic DNS update client for .bit domains.

Donations: BTC 1EcUWRa9H6ZuWPkF3BDj6k4k1vCgv41ab8 ; NMC NFqbaS7ReiQ9MBmsowwcDSmp4iDznjmEh5


Thu Oct 19, 2017 5:16 am
Profile
Display posts from previous:  Sort by  
Reply to topic   [ 12 posts ]  Go to page Previous  1, 2

Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
cron
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group.
Designed by STSoftware for PTF.