aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--doc/developer-notes.md73
1 files changed, 73 insertions, 0 deletions
diff --git a/doc/developer-notes.md b/doc/developer-notes.md
index 797507cd3e..fd75ada79f 100644
--- a/doc/developer-notes.md
+++ b/doc/developer-notes.md
@@ -495,3 +495,76 @@ Git and GitHub tips
This will add an `upstream-pull` remote to your git repository, which can be fetched using `git fetch --all`
or `git fetch upstream-pull`. Afterwards, you can use `upstream-pull/NUMBER/head` in arguments to `git show`,
`git checkout` and anywhere a commit id would be acceptable to see the changes from pull request NUMBER.
+
+RPC interface guidelines
+--------------------------
+
+A few guidelines for introducing and reviewing new RPC interfaces:
+
+- Method naming: use consecutive lower-case names such as `getrawtransaction` and `submitblock`
+
+ - *Rationale*: Consistency with existing interface.
+
+- Argument naming: use snake case `fee_delta` (and not, e.g. camel case `feeDelta`)
+
+ - *Rationale*: Consistency with existing interface.
+
+- Use the JSON parser for parsing, don't manually parse integers or strings from
+ arguments unless absolutely necessary.
+
+ - *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.
+ JSON already supports nested data structures, no need to re-invent the wheel.
+
+ - *Exception*: AmountToValue can parse amounts as string. This was introduced because many JSON
+ parsers and formatters hard-code handling decimal numbers as floating point
+ values, resulting in potential loss of precision. This is unacceptable for
+ monetary values. **Always** use `AmountToValue` and `ValueToAmount` when
+ inputting or outputting monetary values. The only exceptions to this are
+ `prioritisetransaction` and `getblocktemplate` because their interface
+ is specified as-is in BIP22.
+
+- 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.
+
+ - *Rationale*: Avoids surprises when switching to name-based arguments. Missing name-based arguments
+ are passed as 'null'.
+
+ - *Exception*: Many legacy exceptions to this exist, one of the worst ones is
+ `getbalance` which follows a completely different code path based on the
+ number of arguments. We are still in the process of cleaning these up. Do not introduce
+ new ones.
+
+- Try not to overload methods on argument type. E.g. don't make `getblock(true)` and `getblock("hash")`
+ do different things.
+
+ - *Rationale*: This is impossible to use with `bitcoin-cli`, and can be surprising to users.
+
+ - *Exception*: Some RPC calls can take both an `int` and `bool`, most notably when a bool was switched
+ to a multi-value, or due to other historical reasons. **Always** have false map to 0 and
+ true to 1 in this case.
+
+- Don't forget to fill in the argument names correctly in the RPC command table.
+
+ - *Rationale*: If not, the call can not be used with name-based arguments.
+
+- Set okSafeMode in the RPC command table to a sensible value: safe mode is when the
+ blockchain is regarded to be in a confused state, and the client deems it unsafe to
+ do anything irreversible such as send. Anything that just queries should be permitted.
+
+ - *Rationale*: Troubleshooting a node in safe mode is difficult if half the
+ RPCs don't work.
+
+- Add every non-string RPC argument `(method, idx, name)` to the table `vRPCConvertParams` in `rpc/client.cpp`.
+
+ - *Rationale*: `bitcoin-cli` and the GUI debug console use this table to determine how to
+ convert a plaintext command line to JSON. If the types don't match, the method can be unusable
+ from there.
+
+- A RPC method must either be a wallet method or a non-wallet method. Do not
+ introduce new methods such as `getinfo` and `signrawtransaction` that differ
+ in behavior based on presence of a wallet.
+
+ - *Rationale*: as well as complicating the implementation and interfering
+ with the introduction of multi-wallet, wallet and non-wallet code should be
+ separated to avoid introducing circular dependencies between code units.