Live data from Hacker News

Show HN: Mako – a full Bitcoin implementation in C

github.com

41–50 of 129 posts

Re: Show HN: Mako – a full Bitcoin implementation in C

#42
post #5
post #4

Some of this C code looks extremely obfuscated. Not a fan.

Do you mind elaborating? I obsess over clean code, so any criticism is appreciated.

I wasn't originally going to look at it but this claim got me intrigued. I'd say compared to a lot of C code I've seen, yeah, you aren't lying, the average cleanliness is so clean it's sick ;) Well done!

Perhaps the GP means the general crypto nature of the project lends to opaqueness. For example, I randomly clicked into https://github.com/chjj/mako/blob/master/src/crypto/chacha20... and one could ask where the magic numbers on lines 41-44 come from. Maybe it's explained in the references, or just specified without explanation as part of the protocol. I looked at the reference implementation at https://cr.yp.to/streamciphers/timings/estreambench/submissi... which has the equivalent line at L66. After finding the implementation of U8TO32_LITTLE which does some bitwise-or'ing and shifting of the first 4 items ("expa" for either sigma or tau), in Lisp I quickly verified:

       (format nil "0x~x"
               (logior (char-code #\e) (ash (char-code #\x) 8) (ash (char-code #\p) 16) (ash (char-code #\a) 24)))
    ;-> "0x61707865"
So perhaps an argument could be made that your implementation could be slightly cleaner by using those sigma/tau constants instead of the magic numbers for the first four entries in the initial state, which would show how they're related, but that doesn't really take away the magic-ness of them, just moves the question elsewhere to why those magic strings. And it wouldn't surprise me if the answer is just the strings were arbitrarily chosen; the sigma/tau naming in the reference suggests a common domain convention to me (I'm not a crypto guy though so I don't know).

Still overall this is a minor point, it looks like a clean piece of code (especially given what I'm used to seeing from browsing other bits of crypto code here and there) and random sampling of other parts of the program are at least as nice and nicer, especially when they're not doing anything complicated, which I've seen plenty of C code make a mess of. While it might not always be clear why something is done, it's at least clear what's going on and where to jump for more context, what the interfaces are, and things are well-named such that I could probably go find references for some of the whys if they actually needed to be answered. e.g. in mempool.c's btc_mempool_verify, I have no clue what "Annoying process known as sigops counting" refers to, but even without that comment, the simple if condition's pieces are well-named so I could go search for more info on sigops, which I wouldn't even expect to be part of the code (at least here) anyway.

(Edit: It also occurs to me that a low ratio of comments and explanation to code could also be what is meant by obfuscation. To me that's not a large part of the cleanliness concept, though it does factor into other qualities. I consider the sqlite codebase to be some of the most beautiful C code on the planet, wonderfully documented and tested, and as a whole its beauty more than makes up for some stylistic or organizational quirks I don't exactly like. Not to say that style is totally unimportant, but neither this nor sqlite are exemplars of ugly C with lots of insane typedefs, super macros, inconsistencies, syntax abuses everywhere, and mngld_nms like vowels cost $100 a pop. For a project I consider "not so good" C code, I reference Enlightenment Foundation Libraries...)

Re: Show HN: Mako – a full Bitcoin implementation in C

#43
post #34

Anything interesting on your choice of project name?

Its placeholder name was originally "libsatoshi", but I was worried that would cause confusion between this project and bitcoin core. I spent the past few days thinking of names. Mako came to mind because I had recently been playing through the original FF7 (maybe the first time in ~10 years). It's short, memorable, looks cool. It checked all the boxes. On top of that, pretty much every name involving the words "btc"…

Mako is also a fairly popular python template library.

https://www.makotemplates.org/

Re: Show HN: Mako – a full Bitcoin implementation in C

#44
post #5

Earlier quoted context omitted.

Do you mind elaborating? I obsess over clean code, so any criticism is appreciated.

It looks super clean to me, the only exception being for the love of all that's good and holy always brace your one-line if statements and loops. You don't want your own "goto fail" do you? ;) [1] I think you did a great job. [1] https://nakedsecurity.sophos.com/2014/02/24/anatomy-of-a-got...

> It looks super clean to me, the only exception being for the love of all that's good and holy always brace your one-line if statements and loops.

> You don't want your own "goto fail" do you? ;) [1]

Ah, that was my style 7+ years ago. Then I started regularly contributing to an open source project which did not add curly braces on one-liners. I wanted to match the style of the project despite it feeling unnatural to me. Within a few weeks, my style was changed forever (for better or for worse).

> I think you did a great job.

Thank you.

Re: Show HN: Mako – a full Bitcoin implementation in C

#45
post #8

This looks like a herculean effort over just the past few months. Congratulations on hitting this milestone and hopefully you can take a breather and tie up loose ends at a more comfortable pace. As much as HN has changed over the years I still think this is a place where there are a lot of people who can appreciate how you feel right now. Nicely done!

Agreed! I'm confused by the number of comments that suggest this was a waste of time because other potentially similar implementations exist. There could be a hundred of these and I'd still be interested in looking at them.

Re: Show HN: Mako – a full Bitcoin implementation in C

#46
post #5

Earlier quoted context omitted.

Do you mind elaborating? I obsess over clean code, so any criticism is appreciated.

It looks super clean to me, the only exception being for the love of all that's good and holy always brace your one-line if statements and loops. You don't want your own "goto fail" do you? ;) [1] I think you did a great job. [1] https://nakedsecurity.sophos.com/2014/02/24/anatomy-of-a-got...

Won't necessarily work:

  if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
  {
    goto fail;
  }
  {
    goto fail;  /* MISTAKE! THIS BLOCK SHOULD NOT BE HERE */
  }
Copy and paste bugs can play out in any number of ways.

Newer GCC will catch the goto fail by noting that the indentation is wrong.

  $ gcc-11 -Wall gotofail.c 
  gotofail.c: In function ‘main’:
  gotofail.c:5:3: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
      5 |   if (argc > 42)
        |   ^~
  gotofail.c:7:5: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
      7 |     goto fail;
        |     ^~~~

  $ cat gotofail.c 
  #include 
  
  int main(int argc, char **argv)
  {
    if (argc > 42)
      goto fail;
      goto fail;

    return 0;
  fail:
    return EXIT_FAILURE;
  }
Of course, it could be that the indentation is not wrong. If somneone runs the code through some automatic formatting, the problem will not then be diagnosable that way.

Sadly, if I fix the indentation then:

  $ gcc-11 -Wall -Wunreachable-code -O3 gotofail.c 
  $ # silence
even though the return 0; is not reachable.

Still, I think I'm going to stick with "brace the else part if the then part is braced, and vice versa*.

Re: Show HN: Mako – a full Bitcoin implementation in C

#47
post #45
post #8

This looks like a herculean effort over just the past few months. Congratulations on hitting this milestone and hopefully you can take a breather and tie up loose ends at a more comfortable pace. As much as HN has changed over the years I still think this is a place where there are a lot of people who can appreciate how you feel right now. Nicely done!

Agreed! I'm confused by the number of comments that suggest this was a waste of time because other potentially similar implementations exist. There could be a hundred of these and I'd still be interested in looking at them.

Disregard those comments. There will always be critics wondering why you don't just use floppies taped to a carrier pigeon instead of email. We are nerds.

Re: Show HN: Mako – a full Bitcoin implementation in C

#48
post #5

Earlier quoted context omitted.

Do you mind elaborating? I obsess over clean code, so any criticism is appreciated.

> I obsess over clean code Yep, it shows. I got curious over the GP's comment, and I spot checked json.c[1] and json.h[2]. They're cleanly written, your data structures reflect your use and I can get an idea of what does what through the code alone. Trying to figure out what could be considered obscure. For example would if (!btc_amount_import(&x, obj->u.string.ptr)) return 0; be considered obscure? Cause if you know…

Not sure. I suppose I could add a style or coding convention guide somewhere in the repo to code that may seem confusing initially.

Basic rules are: return values on the left side of params, and functions generally return a boolean for success or failure.

The function you mention is parsing a fixed-point integer string (from a json string) and returning an int64_t. It will return 0 if it's not a syntactically valid integer, or if there is some kind of overflow, etc.

Re: Show HN: Mako – a full Bitcoin implementation in C

#49
post #34

Anything interesting on your choice of project name?

Its placeholder name was originally "libsatoshi", but I was worried that would cause confusion between this project and bitcoin core. I spent the past few days thinking of names. Mako came to mind because I had recently been playing through the original FF7 (maybe the first time in ~10 years). It's short, memorable, looks cool. It checked all the boxes. On top of that, pretty much every name involving the words "btc"…

Mako was the thing that Shinra almost destroyed the planet trying to extract, so you may end up opening the project up to a lot of Barret+Bitcoin memes. That could be a win!
Post reply on HN