LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

doc: Fix grammar and punctuation in developer notes

Browse source 
This pull request is regarding minor grammar and punctuation errors in the developer notes. There were no modifications to the existing code, only alterations to fix the grammar and punctuation in the text.

Update developer-notes.md

Update developer-notes.md

Revert "Update developer-notes.md"

This reverts commit dfeb0bacb054ed24766f8af7bae0c0166b0cb4cb.
This commit is contained in:
Kristian Kramer 2019-08-01 20:40:27 -06:00
parent 5639d71a07
commit b2ea20d330
1 changed files with 76 additions and 79 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

155
doc/developer-notes.md
View file

@ -62,7 +62,7 @@ tool to clean up patches automatically before submission.
- Braces on the same line for everything else. - Braces on the same line for everything else.
- 4 space indentation (no tabs) for every block except namespaces. - 4 space indentation (no tabs) for every block except namespaces.
- No indentation for `public`/`protected`/`private` or for `namespace`. - No indentation for `public`/`protected`/`private` or for `namespace`.
- No extra spaces inside parenthesis; don't do ( this ) - No extra spaces inside parenthesis; don't do ( this ).
- No space after function names; one space after `if`, `for` and `while`. - No space after function names; one space after `if`, `for` and `while`.
- If an `if` only has a single-statement `then`-clause, it can appear - If an `if` only has a single-statement `then`-clause, it can appear
on the same line as the `if`, without braces. In every other case, on the same line as the `if`, without braces. In every other case,
@ -72,12 +72,12 @@ tool to clean up patches automatically before submission.
- **Symbol naming conventions**. These are preferred in new code, but are not - **Symbol naming conventions**. These are preferred in new code, but are not
required when doing so would need changes to significant pieces of existing required when doing so would need changes to significant pieces of existing
code. code.
- Variable (including function arguments) and namespace names are all lowercase, and may use `_` to - Variable (including function arguments) and namespace names are all lowercase and may use `_` to
separate words (snake_case). separate words (snake_case).
- Class member variables have a `m_` prefix. - Class member variables have a `m_` prefix.
- Global variables have a `g_` prefix. - Global variables have a `g_` prefix.
- Constant names are all uppercase, and use `_` to separate words. - Constant names are all uppercase, and use `_` to separate words.
- Class names, function names and method names are UpperCamelCase - Class names, function names, and method names are UpperCamelCase
(PascalCase). Do not prefix class names with `C`. (PascalCase). Do not prefix class names with `C`.
- Test suite naming convention: The Boost test suite in file - Test suite naming convention: The Boost test suite in file
`src/test/foo_tests.cpp` should be named `foo_tests`. Test suite names `src/test/foo_tests.cpp` should be named `foo_tests`. Test suite names
@ -134,6 +134,7 @@ Bitcoin Core uses [Doxygen](http://www.doxygen.nl/) to generate its official doc
Use Doxygen-compatible comment blocks for functions, methods, and fields. Use Doxygen-compatible comment blocks for functions, methods, and fields.
For example, to describe a function use: For example, to describe a function use:
```c++ ```c++
/** /**
* ... text ... * ... text ...
@ -143,11 +144,12 @@ For example, to describe a function use:
*/ */
bool function(int arg1, const char *arg2) bool function(int arg1, const char *arg2)
``` ```
A complete list of `@xxx` commands can be found at http://www.doxygen.nl/manual/commands.html. A complete list of `@xxx` commands can be found at http://www.doxygen.nl/manual/commands.html.
As Doxygen recognizes the comments by the delimiters (`/**` and `*/` in this case), you don't As Doxygen recognizes the comments by the delimiters (`/**` and `*/` in this case), you don't
*need* to provide any commands for a comment to be valid; just a description text is fine. *need* to provide any commands for a comment to be valid; just a description text is fine.
To describe a class use the same construct above the class definition: To describe a class, use the same construct above the class definition:
```c++ ```c++
/** /**
* Alerts are for notifying old versions if they become too obsolete and * Alerts are for notifying old versions if they become too obsolete and
@ -189,7 +191,7 @@ but the above styles are favored.
Documentation can be generated with `make docs` and cleaned up with `make clean-docs`. The resulting files are located in `doc/doxygen/html`; open `index.html` to view the homepage. Documentation can be generated with `make docs` and cleaned up with `make clean-docs`. The resulting files are located in `doc/doxygen/html`; open `index.html` to view the homepage.
Before running `make docs`, you will need to install dependencies `doxygen` and `dot`. For example, on MacOS via Homebrew: Before running `make docs`, you will need to install dependencies `doxygen` and `dot`. For example, on macOS via Homebrew:
``` ```
brew install doxygen --with-graphviz brew install doxygen --with-graphviz
``` ```
@ -231,7 +233,7 @@ that run in `-regtest` mode.
Bitcoin Core is a multi-threaded application, and deadlocks or other Bitcoin Core is a multi-threaded application, and deadlocks or other
multi-threading bugs can be very difficult to track down. The `--enable-debug` multi-threading bugs can be very difficult to track down. The `--enable-debug`
configure option adds `-DDEBUG_LOCKORDER` to the compiler flags. This inserts configure option adds `-DDEBUG_LOCKORDER` to the compiler flags. This inserts
run-time checks to keep track of which locks are held, and adds warnings to the run-time checks to keep track of which locks are held and adds warnings to the
debug.log file if inconsistencies are detected. debug.log file if inconsistencies are detected.
### Valgrind suppressions file ### Valgrind suppressions file
@ -299,7 +301,7 @@ $ perf record \
-p `pgrep bitcoind` -- sleep 60 -p `pgrep bitcoind` -- sleep 60
``` ```
You could then analyze the results by running You could then analyze the results by running:
```sh ```sh
perf report --stdio | c++filt | less perf report --stdio | c++filt | less
@ -364,7 +366,7 @@ Additional resources:
Locking/mutex usage notes Locking/mutex usage notes
------------------------- -------------------------
The code is multi-threaded, and uses mutexes and the The code is multi-threaded and uses mutexes and the
`LOCK` and `TRY_LOCK` macros to protect data structures. `LOCK` and `TRY_LOCK` macros to protect data structures.
Deadlocks due to inconsistent lock ordering (thread 1 locks `cs_main` and then Deadlocks due to inconsistent lock ordering (thread 1 locks `cs_main` and then
@ -389,7 +391,7 @@ Threads
- ThreadDNSAddressSeed : Loads addresses of peers from the DNS. - ThreadDNSAddressSeed : Loads addresses of peers from the DNS.
- ThreadMapPort : Universal plug-and-play startup/shutdown - ThreadMapPort : Universal plug-and-play startup/shutdown.
- ThreadSocketHandler : Sends/Receives data from peers on port 8333. - ThreadSocketHandler : Sends/Receives data from peers on port 8333.
@ -408,7 +410,7 @@ Threads
Ignoring IDE/editor files Ignoring IDE/editor files
-------------------------- --------------------------
In closed-source environments in which everyone uses the same IDE it is common In closed-source environments in which everyone uses the same IDE, it is common
to add temporary files it produces to the project-wide `.gitignore` file. to add temporary files it produces to the project-wide `.gitignore` file.
However, in open source software such as Bitcoin Core, where everyone uses However, in open source software such as Bitcoin Core, where everyone uses
@ -446,19 +448,19 @@ pay attention to for reviewers of Bitcoin Core code.
General Bitcoin Core General Bitcoin Core
---------------------- ----------------------
- New features should be exposed on RPC first, then can be made available in the GUI - New features should be exposed on RPC first, then can be made available in the GUI.
- *Rationale*: RPC allows for better automatic testing. The test suite for - *Rationale*: RPC allows for better automatic testing. The test suite for
the GUI is very limited the GUI is very limited.
- Make sure pull requests pass Travis CI before merging - Make sure pull requests pass Travis CI before merging.
- *Rationale*: Makes sure that they pass thorough testing, and that the tester will keep passing - *Rationale*: Makes sure that they pass thorough testing, and that the tester will keep passing
on the master branch. Otherwise all new pull requests will start failing the tests, resulting in on the master branch. Otherwise, all new pull requests will start failing the tests, resulting in
confusion and mayhem confusion and mayhem.
- *Explanation*: If the test suite is to be updated for a change, this has to - *Explanation*: If the test suite is to be updated for a change, this has to
be done first be done first.
Wallet Wallet
------- -------
@ -466,13 +468,13 @@ Wallet
- Make sure that no crashes happen with run-time option `-disablewallet`. - Make sure that no crashes happen with run-time option `-disablewallet`.
- *Rationale*: In RPC code that conditionally uses the wallet (such as - *Rationale*: In RPC code that conditionally uses the wallet (such as
`validateaddress`) it is easy to forget that global pointer `pwalletMain` `validateaddress`), it is easy to forget that global pointer `pwalletMain`
can be nullptr. See `test/functional/disablewallet.py` for functional tests can be nullptr. See `test/functional/disablewallet.py` for functional tests
exercising the API with `-disablewallet` exercising the API with `-disablewallet`.
- Include `db_cxx.h` (BerkeleyDB header) only when `ENABLE_WALLET` is set - Include `db_cxx.h` (BerkeleyDB header) only when `ENABLE_WALLET` is set.
- *Rationale*: Otherwise compilation of the disable-wallet build will fail in environments without BerkeleyDB - *Rationale*: Otherwise compilation of the disable-wallet build will fail in environments without BerkeleyDB.
General C++ General C++
------------- -------------
@ -483,26 +485,26 @@ Guidelines](https://isocpp.github.io/CppCoreGuidelines/).
Common misconceptions are clarified in those sections: Common misconceptions are clarified in those sections:
- Passing (non-)fundamental types in the [C++ Core - Passing (non-)fundamental types in the [C++ Core
Guideline](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-conventional) Guideline](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-conventional).
- Assertions should not have side-effects - Assertions should not have side-effects.
- *Rationale*: Even though the source code is set to refuse to compile - *Rationale*: Even though the source code is set to refuse to compile
with assertions disabled, having side-effects in assertions is unexpected and with assertions disabled, having side-effects in assertions is unexpected and
makes the code harder to understand makes the code harder to understand.
- If you use the `.h`, you must link the `.cpp` - If you use the `.h`, you must link the `.cpp`.
- *Rationale*: Include files define the interface for the code in implementation files. Including one but - *Rationale*: Include files define the interface for the code in implementation files. Including one but
not linking the other is confusing. Please avoid that. Moving functions from not linking the other is confusing. Please avoid that. Moving functions from
the `.h` to the `.cpp` should not result in build errors the `.h` to the `.cpp` should not result in build errors.
- Use the RAII (Resource Acquisition Is Initialization) paradigm where possible. For example by using - Use the RAII (Resource Acquisition Is Initialization) paradigm where possible. For example, by using
`unique_ptr` for allocations in a function. `unique_ptr` for allocations in a function.
- *Rationale*: This avoids memory and resource leaks, and ensures exception safety - *Rationale*: This avoids memory and resource leaks, and ensures exception safety.
- Use `MakeUnique()` to construct objects owned by `unique_ptr`s - Use `MakeUnique()` to construct objects owned by `unique_ptr`s.
- *Rationale*: `MakeUnique` is concise and ensures exception safety in complex expressions. - *Rationale*: `MakeUnique` is concise and ensures exception safety in complex expressions.
`MakeUnique` is a temporary project local implementation of `std::make_unique` (C++14). `MakeUnique` is a temporary project local implementation of `std::make_unique` (C++14).
@ -510,27 +512,27 @@ Common misconceptions are clarified in those sections:
C++ data structures C++ data structures
-------------------- --------------------
- Never use the `std::map []` syntax when reading from a map, but instead use `.find()` - Never use the `std::map []` syntax when reading from a map, but instead use `.find()`.
- *Rationale*: `[]` does an insert (of the default element) if the item doesn't - *Rationale*: `[]` does an insert (of the default element) if the item doesn't
exist in the map yet. This has resulted in memory leaks in the past, as well as exist in the map yet. This has resulted in memory leaks in the past, as well as
race conditions (expecting read-read behavior). Using `[]` is fine for *writing* to a map race conditions (expecting read-read behavior). Using `[]` is fine for *writing* to a map.
- Do not compare an iterator from one data structure with an iterator of - Do not compare an iterator from one data structure with an iterator of
another data structure (even if of the same type) another data structure (even if of the same type).
- *Rationale*: Behavior is undefined. In C++ parlor this means "may reformat - *Rationale*: Behavior is undefined. In C++ parlor this means "may reformat
the universe", in practice this has resulted in at least one hard-to-debug crash bug the universe", in practice this has resulted in at least one hard-to-debug crash bug.
- Watch out for out-of-bounds vector access. `&vch[vch.size()]` is illegal, - Watch out for out-of-bounds vector access. `&vch[vch.size()]` is illegal,
including `&vch[0]` for an empty vector. Use `vch.data()` and `vch.data() + including `&vch[0]` for an empty vector. Use `vch.data()` and `vch.data() +
vch.size()` instead. vch.size()` instead.
- Vector bounds checking is only enabled in debug mode. Do not rely on it - Vector bounds checking is only enabled in debug mode. Do not rely on it.
- Initialize all non-static class members where they are defined. - Initialize all non-static class members where they are defined.
If this is skipped for a good reason (i.e., optimization on the critical If this is skipped for a good reason (i.e., optimization on the critical
path), add an explicit comment about this path), add an explicit comment about this.
- *Rationale*: Ensure determinism by avoiding accidental use of uninitialized - *Rationale*: Ensure determinism by avoiding accidental use of uninitialized
values. Also, static analyzers balk about this. values. Also, static analyzers balk about this.
@ -554,12 +556,12 @@ class A
`int8_t`. Do not use bare `char` unless it is to pass to a third-party API. `int8_t`. Do not use bare `char` unless it is to pass to a third-party API.
This type can be signed or unsigned depending on the architecture, which can This type can be signed or unsigned depending on the architecture, which can
lead to interoperability problems or dangerous conditions such as lead to interoperability problems or dangerous conditions such as
out-of-bounds array accesses out-of-bounds array accesses.
- Prefer explicit constructions over implicit ones that rely on 'magical' C++ behavior - Prefer explicit constructions over implicit ones that rely on 'magical' C++ behavior.
- *Rationale*: Easier to understand what is happening, thus easier to spot mistakes, even for those - *Rationale*: Easier to understand what is happening, thus easier to spot mistakes, even for those
that are not language lawyers that are not language lawyers.
Strings and formatting Strings and formatting
------------------------ ------------------------
@ -567,17 +569,17 @@ Strings and formatting
- Be careful of `LogPrint` versus `LogPrintf`. `LogPrint` takes a `category` argument, `LogPrintf` does not. - Be careful of `LogPrint` versus `LogPrintf`. `LogPrint` takes a `category` argument, `LogPrintf` does not.
- *Rationale*: Confusion of these can result in runtime exceptions due to - *Rationale*: Confusion of these can result in runtime exceptions due to
formatting mismatch, and it is easy to get wrong because of subtly similar naming formatting mismatch, and it is easy to get wrong because of subtly similar naming.
- Use `std::string`, avoid C string manipulation functions - Use `std::string`, avoid C string manipulation functions.
- *Rationale*: C++ string handling is marginally safer, less scope for - *Rationale*: C++ string handling is marginally safer, less scope for
buffer overflows and surprises with `\0` characters. Also some C string manipulations buffer overflows, and surprises with `\0` characters. Also, some C string manipulations
tend to act differently depending on platform, or even the user locale tend to act differently depending on platform, or even the user locale.
- Use `ParseInt32`, `ParseInt64`, `ParseUInt32`, `ParseUInt64`, `ParseDouble` from `utilstrencodings.h` for number parsing - Use `ParseInt32`, `ParseInt64`, `ParseUInt32`, `ParseUInt64`, `ParseDouble` from `utilstrencodings.h` for number parsing.
- *Rationale*: These functions do overflow checking, and avoid pesky locale issues. - *Rationale*: These functions do overflow checking and avoid pesky locale issues.
- Avoid using locale dependent functions if possible. You can use the provided - Avoid using locale dependent functions if possible. You can use the provided
[`lint-locale-dependence.sh`](/test/lint/lint-locale-dependence.sh) [`lint-locale-dependence.sh`](/test/lint/lint-locale-dependence.sh)
@ -607,14 +609,14 @@ Strings and formatting
`wcstoll`, `wcstombs`, `wcstoul`, `wcstoull`, `wcstoumax`, `wcswidth`, `wcstoll`, `wcstombs`, `wcstoul`, `wcstoull`, `wcstoumax`, `wcswidth`,
`wcsxfrm`, `wctob`, `wctomb`, `wctrans`, `wctype`, `wcwidth`, `wprintf` `wcsxfrm`, `wctob`, `wctomb`, `wctrans`, `wctype`, `wcwidth`, `wprintf`
- For `strprintf`, `LogPrint`, `LogPrintf` formatting characters don't need size specifiers - For `strprintf`, `LogPrint`, `LogPrintf` formatting characters don't need size specifiers.
- *Rationale*: Bitcoin Core uses tinyformat, which is type safe. Leave them out to avoid confusion - *Rationale*: Bitcoin Core uses tinyformat, which is type safe. Leave them out to avoid confusion.
Variable names Variable names
-------------- --------------
Although the shadowing warning (`-Wshadow`) is not enabled by default (it prevents issues rising Although the shadowing warning (`-Wshadow`) is not enabled by default (it prevents issues arising
from using a different variable with the same name), from using a different variable with the same name),
please name variables so that their names do not shadow variables defined in the source code. please name variables so that their names do not shadow variables defined in the source code.
@ -633,22 +635,20 @@ AddressBookPage::AddressBookPage(Mode _mode)
``` ```
When using nested cycles, do not name the inner cycle variable the same as in When using nested cycles, do not name the inner cycle variable the same as in
upper cycle etc. the upper cycle, etc.
Threads and synchronization Threads and synchronization
---------------------------- ----------------------------
- Build and run tests with `-DDEBUG_LOCKORDER` to verify that no potential - Build and run tests with `-DDEBUG_LOCKORDER` to verify that no potential
deadlocks are introduced. As of 0.12, this is defined by default when deadlocks are introduced. As of 0.12, this is defined by default when
configuring with `--enable-debug` configuring with `--enable-debug`.
- When using `LOCK`/`TRY_LOCK` be aware that the lock exists in the context of - When using `LOCK`/`TRY_LOCK` be aware that the lock exists in the context of
the current scope, so surround the statement and the code that needs the lock the current scope, so surround the statement and the code that needs the lock
with braces with braces.
OK: OK:
```c++ ```c++
{ {
TRY_LOCK(cs_vNodes, lockNodes); TRY_LOCK(cs_vNodes, lockNodes);
@ -657,7 +657,6 @@ Threads and synchronization
``` ```
Wrong: Wrong:
```c++ ```c++
TRY_LOCK(cs_vNodes, lockNodes); TRY_LOCK(cs_vNodes, lockNodes);
{ {
@ -679,13 +678,11 @@ Scripts
`#!/usr/bin/env bash` searches the user's PATH to find the bash binary. `#!/usr/bin/env bash` searches the user's PATH to find the bash binary.
OK: OK:
```bash ```bash
#!/usr/bin/env bash #!/usr/bin/env bash
``` ```
Wrong: Wrong:
```bash ```bash
#!/bin/bash #!/bin/bash
``` ```
@ -694,9 +691,9 @@ Source code organization
-------------------------- --------------------------
- Implementation code should go into the `.cpp` file and not the `.h`, unless necessary due to template usage or - Implementation code should go into the `.cpp` file and not the `.h`, unless necessary due to template usage or
when performance due to inlining is critical when performance due to inlining is critical.
- *Rationale*: Shorter and simpler header files are easier to read, and reduce compile time - *Rationale*: Shorter and simpler header files are easier to read and reduce compile time.
- Use only the lowercase alphanumerics (`a-z0-9`), underscore (`_`) and hyphen (`-`) in source code filenames. - Use only the lowercase alphanumerics (`a-z0-9`), underscore (`_`) and hyphen (`-`) in source code filenames.
@ -714,7 +711,7 @@ Source code organization
- Don't import anything into the global namespace (`using namespace ...`). Use - Don't import anything into the global namespace (`using namespace ...`). Use
fully specified types such as `std::string`. fully specified types such as `std::string`.
- *Rationale*: Avoids symbol conflicts - *Rationale*: Avoids symbol conflicts.
- Terminate namespaces with a comment (`// namespace mynamespace`). The comment - Terminate namespaces with a comment (`// namespace mynamespace`). The comment
should be placed on the same line as the brace closing the namespace, e.g. should be placed on the same line as the brace closing the namespace, e.g.
@ -729,7 +726,7 @@ namespace {
} // namespace } // namespace
``` ```
- *Rationale*: Avoids confusion about the namespace context - *Rationale*: Avoids confusion about the namespace context.
- Use `#include <primitives/transaction.h>` bracket syntax instead of - Use `#include <primitives/transaction.h>` bracket syntax instead of
`#include "primitives/transactions.h"` quote syntax. `#include "primitives/transactions.h"` quote syntax.
@ -752,13 +749,13 @@ namespace {
GUI GUI
----- -----
- Do not display or manipulate dialogs in model code (classes `*Model`) - Do not display or manipulate dialogs in model code (classes `*Model`).
- *Rationale*: Model classes pass through events and data from the core, they - *Rationale*: Model classes pass through events and data from the core, they
should not interact with the user. That's where View classes come in. The converse also should not interact with the user. That's where View classes come in. The converse also
holds: try to not directly access core data structures from Views. holds: try to not directly access core data structures from Views.
- Avoid adding slow or blocking code in the GUI thread. In particular do not - Avoid adding slow or blocking code in the GUI thread. In particular, do not
add new `interfaces::Node` and `interfaces::Wallet` method calls, even if they add new `interfaces::Node` and `interfaces::Wallet` method calls, even if they
may be fast now, in case they are changed to lock or communicate across may be fast now, in case they are changed to lock or communicate across
processes in the future. processes in the future.
@ -777,12 +774,12 @@ Subtrees
Several parts of the repository are subtrees of software maintained elsewhere. Several parts of the repository are subtrees of software maintained elsewhere.
Some of these are maintained by active developers of Bitcoin Core, in which case changes should probably go Some of these are maintained by active developers of Bitcoin Core, in which case changes should probably go
directly upstream without being PRed directly against the project. They will be merged back in the next directly upstream without being PRed directly against the project. They will be merged back in the next
subtree merge. subtree merge.
Others are external projects without a tight relationship with our project. Changes to these should also Others are external projects without a tight relationship with our project. Changes to these should also
be sent upstream but bugfixes may also be prudent to PR against Bitcoin Core so that they can be integrated be sent upstream, but bugfixes may also be prudent to PR against Bitcoin Core so that they can be integrated
quickly. Cosmetic changes should be purely taken upstream. quickly. Cosmetic changes should be purely taken upstream.
There is a tool in `test/lint/git-subtree-check.sh` to check a subtree directory for consistency with There is a tool in `test/lint/git-subtree-check.sh` to check a subtree directory for consistency with
its upstream repository. its upstream repository.
@ -812,11 +809,11 @@ you must be aware of.
### File Descriptor Counts ### File Descriptor Counts
In most configurations we use the default LevelDB value for `max_open_files`, In most configurations, we use the default LevelDB value for `max_open_files`,
which is 1000 at the time of this writing. If LevelDB actually uses this many which is 1000 at the time of this writing. If LevelDB actually uses this many
file descriptors it will cause problems with Bitcoin's `select()` loop, because file descriptors, it will cause problems with Bitcoin's `select()` loop, because
it may cause new sockets to be created where the fd value is >= 1024. For this it may cause new sockets to be created where the fd value is >= 1024. For this
reason, on 64-bit Unix systems we rely on an internal LevelDB optimization that reason, on 64-bit Unix systems, we rely on an internal LevelDB optimization that
uses `mmap()` + `close()` to open table files without actually retaining uses `mmap()` + `close()` to open table files without actually retaining
references to the table file descriptors. If you are upgrading LevelDB, you must references to the table file descriptors. If you are upgrading LevelDB, you must
sanity check the changes to make sure that this assumption remains valid. sanity check the changes to make sure that this assumption remains valid.
@ -841,14 +838,14 @@ details.
It is possible for LevelDB changes to inadvertently change consensus It is possible for LevelDB changes to inadvertently change consensus
compatibility between nodes. This happened in Bitcoin 0.8 (when LevelDB was compatibility between nodes. This happened in Bitcoin 0.8 (when LevelDB was
first introduced). When upgrading LevelDB you should review the upstream changes first introduced). When upgrading LevelDB, you should review the upstream changes
to check for issues affecting consensus compatibility. to check for issues affecting consensus compatibility.
For example, if LevelDB had a bug that accidentally prevented a key from being For example, if LevelDB had a bug that accidentally prevented a key from being
returned in an edge case, and that bug was fixed upstream, the bug "fix" would returned in an edge case, and that bug was fixed upstream, the bug "fix" would
be an incompatible consensus change. In this situation the correct behavior be an incompatible consensus change. In this situation, the correct behavior
would be to revert the upstream fix before applying the updates to Bitcoin's would be to revert the upstream fix before applying the updates to Bitcoin's
copy of LevelDB. In general you should be wary of any upstream changes affecting copy of LevelDB. In general, you should be wary of any upstream changes affecting
what data is returned from LevelDB queries. what data is returned from LevelDB queries.
Scripted diffs Scripted diffs
@ -867,7 +864,7 @@ To create a scripted-diff:
- `-BEGIN VERIFY SCRIPT-` - `-BEGIN VERIFY SCRIPT-`
- `-END VERIFY SCRIPT-` - `-END VERIFY SCRIPT-`
The scripted-diff is verified by the tool `test/lint/commit-script-check.sh`. The tool's default behavior when supplied The scripted-diff is verified by the tool `test/lint/commit-script-check.sh`. The tool's default behavior, when supplied
with a commit is to verify all scripted-diffs from the beginning of time up to said commit. Internally, the tool passes with a commit is to verify all scripted-diffs from the beginning of time up to said commit. Internally, the tool passes
the first supplied argument to `git rev-list --reverse` to determine which commits to verify script-diffs for, ignoring the first supplied argument to `git rev-list --reverse` to determine which commits to verify script-diffs for, ignoring
commits that don't conform to the commit message format described above. commits that don't conform to the commit message format described above.
@ -900,23 +897,23 @@ RPC interface guidelines
A few guidelines for introducing and reviewing new RPC interfaces: A few guidelines for introducing and reviewing new RPC interfaces:
- Method naming: use consecutive lower-case names such as `getrawtransaction` and `submitblock` - Method naming: use consecutive lower-case names such as `getrawtransaction` and `submitblock`.
- *Rationale*: Consistency with existing interface. - *Rationale*: Consistency with the existing interface.
- Argument naming: use snake case `fee_delta` (and not, e.g. camel case `feeDelta`) - Argument naming: use snake case `fee_delta` (and not, e.g. camel case `feeDelta`)
- *Rationale*: Consistency with existing interface. - *Rationale*: Consistency with the existing interface.
- Use the JSON parser for parsing, don't manually parse integers or strings from - Use the JSON parser for parsing, don't manually parse integers or strings from
arguments unless absolutely necessary. arguments unless absolutely necessary.
- *Rationale*: Introduces hand-rolled string manipulation code at both the caller and callee sites, - *Rationale*: Introduces hand-rolled string manipulation code at both the caller and callee sites,
which is error prone, and it is easy to get things such as escaping wrong. which is error-prone, and it is easy to get things such as escaping wrong.
JSON already supports nested data structures, no need to re-invent the wheel. JSON already supports nested data structures, no need to re-invent the wheel.
- *Exception*: AmountFromValue can parse amounts as string. This was introduced because many JSON - *Exception*: AmountFromValue can parse amounts as string. This was introduced because many JSON
parsers and formatters hard-code handling decimal numbers as floating point parsers and formatters hard-code handling decimal numbers as floating-point
values, resulting in potential loss of precision. This is unacceptable for values, resulting in potential loss of precision. This is unacceptable for
monetary values. **Always** use `AmountFromValue` and `ValueFromAmount` when monetary values. **Always** use `AmountFromValue` and `ValueFromAmount` when
inputting or outputting monetary values. The only exceptions to this are inputting or outputting monetary values. The only exceptions to this are
@ -925,7 +922,7 @@ A few guidelines for introducing and reviewing new RPC interfaces:
- Missing arguments and 'null' should be treated the same: as default values. If there is no - Missing arguments and 'null' should be treated the same: as default values. If there is no
default value, both cases should fail in the same way. The easiest way to follow this default value, both cases should fail in the same way. The easiest way to follow this
guideline is detect unspecified arguments with `params[x].isNull()` instead of guideline is to detect unspecified arguments with `params[x].isNull()` instead of
`params.size() <= x`. The former returns true if the argument is either null or missing, `params.size() <= x`. The former returns true if the argument is either null or missing,
while the latter returns true if is missing, and false if it is null. while the latter returns true if is missing, and false if it is null.
@ -959,7 +956,7 @@ A few guidelines for introducing and reviewing new RPC interfaces:
from there. from there.
- A RPC method must either be a wallet method or a non-wallet method. Do not - A RPC method must either be a wallet method or a non-wallet method. Do not
introduce new methods that differ in behavior based on presence of a wallet. introduce new methods that differ in behavior based on the presence of a wallet.
- *Rationale*: as well as complicating the implementation and interfering - *Rationale*: as well as complicating the implementation and interfering
with the introduction of multi-wallet, wallet and non-wallet code should be with the introduction of multi-wallet, wallet and non-wallet code should be
@ -967,7 +964,7 @@ A few guidelines for introducing and reviewing new RPC interfaces:
- Try to make the RPC response a JSON object. - Try to make the RPC response a JSON object.
- *Rationale*: If a RPC response is not a JSON object then it is harder to avoid API breakage if - *Rationale*: If a RPC response is not a JSON object, then it is harder to avoid API breakage if
new data in the response is needed. new data in the response is needed.
- Wallet RPCs call BlockUntilSyncedToCurrentChain to maintain consistency with - Wallet RPCs call BlockUntilSyncedToCurrentChain to maintain consistency with