Please login or register.

Documentation and cleanup of source code

What

Clean up and properly document (doxygen) the monero source code, and handle any merges necessary to do so seamlessly with other concurrent development efforts.

Who

I'm Thomas Winget, and I've been contributing to the Monero codebase since around June 2014. My largest contribution is the migration from storing the blockchain in a static, binary format to storing the blockchain using a database API, as well as two BlockchainDB implementations (LMDB and BerkeleyDB). Other contributions of mine can be found via the network graph on github or simply looking at my fork.

As a side effect of the work I've done already on Monero, I'm rather familiar with most of the codebase. This puts me in a good position to write down how everything works and fits together, as I've had to sort that out already for many parts.

As a side effect of this effort, I will further cement the knowledge I have of the codebase and gain knowledge in areas I'm less familiar with, enabling me to efficiently implement new features in the future and guide others who may have questions regarding a specific module. In addition, documenting the code will function as a sort of code review, allowing the possibility of spotting any bugs (minor or otherwise).

Why

In my time developing on Monero, there have been countless instances where I need to reference what another piece of code does in order to work with it, and having all the code documented properly in an easily accessible manner (doxygen) would be helpful for that for anyone working in Monero's codebase. I have also come across areas where the code could be made more efficient or clear without changing any functionality, as well as many typographical errors (though these are minor, I think we can all agree they're annoying if nothing else).

Documentation is by far not a glamorous task (probably why it's a bit sparse in Monero), but every development effort benefits immensely from properly documented code.

Proposal and Milestones

I would like to spend about 20 hours per week working on the above, at a rate of 45 XMR per hour (just over $20/hr USD at the time of writing this), paid out every week. I am unsure whether or not 80 hours of work will be enough to cover the entire codebase. There are over 70,000 lines of source in the src/ folder alone, and there is code in other folders that brings the total to over 100k, so this is no small task!

Milestones would be as follows:

  • First 20 hours (900/3600 XMR)
  • Second 20 hours (1800/3600 XMR)
  • Third 20 hours (2700/3600 XMR)
  • Fourth 20 hours (3600/3600 XMR)

After each milestone (and during), I will be available for comment, so anything that seems unclear in my documentation can be addressed and dealt with accordingly.

Replies: 37
tewinget posted 8 years ago Weight: -215 | Link [ + ]

Well that was funded far more aggressively than I had anticipated! Thanks to everyone for the support, I'll get started right away!

Lloydimiller4 posted 8 years ago Replies: 1 | Weight: -215 | Link [ + ]

Damnit, I'm too late to donate any.... I hope we get a few more Devs looking for more funding :D

Great job everyong, getting this funded in one day is proof that our community cares.

rocco edited 8 years ago Weight: -213 | Link [ + ]

respect to all the donators and also to tewinget for all his past and future work. good job everyone

Reply to: Lloydimiller4
tewinget edited 8 years ago Weight: -213 | Link [ + ]

Given the success of this, I'm eager to do more! What I think I'll do over the next week or two is throw a few ideas in either the open tasks section or the ideas section and see which one(s) is most popular (a weak proxy for most worthy), and go from there.

tewinget edited 8 years ago Replies: 2 | Weight: -192 | Link [ + ]

Update #1:

First 20 hours completed. Progress can be seen on the new branch on my fork.

As of now, to the best of my knowledge, all of the comments in src/cryptonote_core/blockchain.{h,cpp} accurately reflect reality. There are also many places where the code could be more clear or more organized which I have outlined with FIXME tags (and most with how to fix/what to change, rather than just "fix pl0x"). There are many pieces of code I've labelled "candidate for removal", but not yet removed, as I want to organize my commits by purpose. That may be overkill, I'm not sure...

I'm not sure if I've struck the right balance between thorough and anal, so I'll leave that open to comment. Also, of course, if anything seems unclear please let me know!

I've compiled a to-do list as I went along, and I'll paste it at the end of this comment. It's largely broad strokes (or just reflections of FIXMEs), but if any of it seems out-of-scope for this...project?...feel free to say so.

I apologize that I'm almost a week late; I definitely did not expect this to get funded in a day, so I had other irons in other fires to contend with. Look for the next update next Thursday or Friday.

Anyway, looking forward to continuing. Going forward I'll probably go through and execute on the FIXMEs so far before moving on to document more, but if you all would prefer the latter, that's also totally fine.


To-do list

General: - Discuss updating function names to better reflect their purpose -- emphasis on discuss :)

  • Fix inconsistent indentation.

  • evaluate return values of all functions (lots of bool return values not being used, etc.) related to this, consider throwing rather than returning empty containers where a container is the return type

src/

  • cryptonote_core/

    • cryptonote_core.{h,cpp}

      • move BlockchainDB initialization and DB-specific code from cryptonote_core.* to better places
    • blockchain.{h,cpp}

      • remove remaining defunct blockchain_storage code from blockchain.*

      • evaluate locking mechanism and possibility of separate reader/writer locks

grep -rn FIXME src/ <-- It seems nobody else really uses FIXME, so that's an easy way to tag TODO-like things which are not feature additions but rather (possible or necessary) corrections.

Reply to: tewinget
Kazuki posted 8 years ago Weight: -195 | Link [ + ]

great work and it will make future contributors more willing to improve monero.

Reply to: tewinget
link posted 8 years ago Weight: -194 | Link [ + ]

nice job tewinget! :D

dEBRUYNE edited 8 years ago Replies: 1 | Weight: -170 | Link [ - ]

How's it going?

Reply to: dEBRUYNE
tewinget posted 8 years ago Replies: 1 | Weight: -170 | Link [ - ]

It's going, but tedious. I'll post a proper update tomorrow, probably evening EST.

Reply to: tewinget dEBRUYNE
Gingeropolous posted 8 years ago Replies: 1 | Weight: -170 | Link [ - ]

How could this possibly be tedious?

Your fighting the good fight sir. Thank you thank you.

Reply to: Gingeropolous tewinget dEBRUYNE
dEBRUYNE posted 8 years ago Weight: -170 | Link [ - ]

+1 and thanks for the reply! Looking forward to the update.

tewinget edited 8 years ago Replies: 1 | Weight: -167 | Link [ - ]

Update #2:

Second 20 hours completed. Progress can be seen on the branch on my fork.

Rather than retype all of the commit messages here, you'll find a paste of "git log --stat" below, or at least hopefully you will, this preview thing hates me.

Please feel free to have a look through the generated Doxygen and let me know if anything seems unclear or incorrect (including Doxygen I haven't touched yet, it all needs to be correct eventually, and if you spot something I may as well know about it!)

Reply to: tewinget
dEBRUYNE posted 8 years ago Weight: -155 | Link [ - ]

Great work! You can see his pull request here -> https://github.com/monero-project/bitmonero/pull/434

dEBRUYNE posted 8 years ago Replies: 1 | Weight: -119 | Link [ - ]

How is it going? Also, was the 4x20 hours meant as consecutive? Or just arbitrary. I am looking forward to hearing some new ideas as well btw :)

Reply to: dEBRUYNE
tewinget posted 8 years ago Replies: 1 | Weight: -115 | Link [ - ]

It was initially meant to be consecutive or close to. I've had some other things come up, but I look to finish out the remaining 40 hours within a week or so.