Hacker News Reader

Comment by McGlockenshire | Hacker News Reader

The code is ... interesting. Smells organic, not designed. Comments are rare but usually useful. Highly coupled. Static methods everywhere. Violates SOLID principles. Basically, ignores current best practices.

Clearly not designed for any sort of automated testing, which should be the first damn thing you do when there's any sort of money involved. Hell, even when there isn't money involved.

We'd already guessed that last bit, given the previous mentions of them having no testing/staging/QA environment.

View on HN

Way too much schadenfreude given Gox's history.

Although contrary to popular belief Gox was never a Magic exchange, they were a Bitcoin startup at a time when Bitcoin was not much more than internet lols and pizza deliveries. The first thing you do when hacking together a stupid exchange for a joke e-currency isn't writing unit tests. You just write the code and blast it up on a domain you had lying around for a different project.

Cue runaway success, a company sale or two, scaling issues with complicated technology and not much precedent legal or otherwise, and this is what you get.

There is nothing at all surprising about this code. And it seems Gox's problems were much more deeply rooted than the subjective non-compliance of their code with "best practices".

> contrary to popular belief Gox was never a Magic exchange

The Wayback Machine disagrees.

http://web.archive.org/web/20070701000000*/http://mtgox.com

Whether it was open as one is unclear, but that was the obvious intent here.

Also, remember that the current owners aren't the original owners. This code is wrong in design but new in attempted style, made or majorly updated within the past year or two (based on the language features used).

My understanding is that while the domain was originally purchased for a MtG exchange, it was never used for this purpose. I've yet to see proof that a MtG exchange has existed on mtgox.com

McCaleb has said by email that MtGox was at least at some point used for trading or buying cards.

> The first thing you do when hacking together a stupid exchange for a joke e-currency isn't writing unit tests.

As soon as you write anything where a bug causes money to change hands, even on a laugh, you write some tests.

No kidding, I would be so freaked out not writing tests the second money is involved. I just don't understand why he didn't bother to hire some help when things got bigger.

That's the part I don't understand – I'd be scared witless at the thought of not having tests for anything moving money out of my accounts. When you have millions in those accounts, even a really expensive developer is cheap insurance…

While it may have not been a MTG card trading site, Karpeles was involved in running MMO servers based around Ragnarok Online before running an exchange. His involvement carried through during MTGox's existence until maybe a year or two ago.

If Mt Gox was ever a MTG card trading site, it would have been before Mark purchased it from Jed McCaleb.

> schadenfreude

Epicaricacy. because you're speaking english.

I'd be less concerned about code quality and technical best practices and more interested in knowing if Mt Gox had any internal notion of preventing common stock market manipulation tricks such as wash trades and chandelier bidding. I bet a lot of bitcoin startups don't know what those terms mean and thereby shouldn't be operating a finance exchange.

Why would you want o stop that? It's like trying to stop kissing your fist before you roll the dice, because other players might be afraid that you are doing magic.

All the rules of traditional stock market are designed to prevent scaring of gamers so they won't leave casino. Bitcoin doesn't care about faint hearted gamers.

Yes, it doesn't look very pretty. It relatively recent code though, making plenty use of php 5.3+ constructs, so you can't even blame it on legacy code.

I've written uglier code. But I didn't write it in places where I had highly motivated adversaries trying to break it with the result that many people's life savings would be irrevocably lost if I had a bug.

no shit, magic the gathering exchange is not coded like paypal.

One class to rule them all.

How is the exception handling? For instance, if something goes wrong when generating a new private key, could they still attempt the transaction but with a null key? (cf the earlier bug that lost 2609 bitcoins.)

Unfortunately I don't know enough about the bitcoin innards to give you an informative answer, and the answer isn't clear from a plain reading of the code.

The leaked code seems to just be an internal API for twiddling wallets. There doesn't seem to be any logic here for either the txid conflict retry bits or the hot/cold transfer bits.

Thanks for the reply. If you look at each method that can return failure, and then look at the code that calls these methods, do the callers check for failure or just keep going with the transaction? (This would both indicate the quality of the code and show a path that could generate bad transactions.)

There are a few (9?) places where "getNullAddr" is called. I assume this would return what was earlier called a "null key." It returns the requested information, except when the new address could not be inserted into the database, in which case it returns false. Less than half of the calls have any failure checking, and a few of those return false as well, and who knows if the things that call those functions have any error checking...

Reply on news.ycombinator.com