|
rippled
|
The XRP Ledger has many and diverse stakeholders, and everyone deserves a chance to contribute meaningful changes to the code that runs the XRPL.
We assume you are familiar with the general practice of making contributions on GitHub. This file includes only special instructions specific to this project.
The following branches exist in the main project repository:
develop: The latest set of unreleased features, and the most common starting point for contributions.release: The latest beta release or release candidate.master: The latest stable release.gh-pages: The documentation for this project, built by Doxygen.The tip of each branch must be signed. In order for GitHub to sign a squashed commit that it builds from your pull request, GitHub must know your verifying key. Please set up signature verification.
In general, external contributions should be developed in your personal fork. Contributions from developers with write permissions should be done in the main repository in a branch with a permitted prefix. Permitted prefixes are:
Regardless of where the branch is created, please open a draft pull request as soon as possible after pushing the branch to Github, to increase visibility, and ease feedback during the development process.
If your contribution is a major feature or breaking change, then you must first write an XRP Ledger Standard (XLS) describing it. Go to XRPL-Standards, choose the next available standard number, and open a discussion with an appropriate title to propose your draft standard.
When you submit a pull request, please link the corresponding XLS in the description. An XLS still in Draft status is considered a work-in-progress and open for discussion. Please allow time for questions, suggestions, and changes to the XLS draft. It is the responsibility of the XLS author to update the draft to match the final implementation when its corresponding pull request is merged, unless the author delegates that responsibility to others.
Any amendment or major RPC change requires either a new XLS or an update to an existing XLS. Neither change will be released (in an amendment's case, marked as Supported::yes) until the corresponding XLS's status is Final.
(Or marking a draft pull request as ready.)
Changes that alter transaction processing must be guarded by an Amendment. All other changes that maintain the existing behavior do not need an Amendment.
Ensure that your code compiles according to the build instructions in `BUILD.md`.
Please write tests for your code. If your test can be run offline, in under 60 seconds, then it can be an automatic test run by xrpld --unittest. Otherwise, it must be a manual test.
If you create new source files, they must be organized as follows:
libxrpl modules, the headers (.h) must go under include/xrpl, and source (.cpp) files must go under src/libxrpl.src/xrpld.src/test.The source must be formatted according to the style guide below.
Header includes must be levelized.
Changes should be usually squashed down into a single commit. Some larger or more complicated change sets make more sense, and are easier to review if organized into multiple logical commits. Either way, all commits should fit the following criteria:
git bisect to find bugs.Refer to "How to Write a Git Commit Message" for general rules on writing a good commit message.
tl;dr
1. Separate subject from body with a blank line.
- Limit the subject line to 50 characters.
- [...]shoot for 50 characters, but consider 72 the hard limit.
- Capitalize the subject line.
- Do not end the subject line with a period.
- Use the imperative mood in the subject line.
- A properly formed Git commit subject line should always be able to complete the following sentence: "If applied, this commit will _your subject line here_".
- Wrap the body at 72 characters.
- Use the body to explain what and why vs. how.
In general, pull requests use develop as the base branch. The exceptions are
release as the base.master as the base.If your changes are not quite ready, but you want to make it easily available for preliminary examination or review, you can create a "Draft" pull request. While a pull request is marked as a "Draft", you can rebase or reorganize the commits in the pull request as desired.
Github pull requests are created as "Ready" by default, or you can mark a "Draft" pull request as "Ready". Once a pull request is marked as "Ready", any changes must be added as new commits. Do not force-push to a branch in a pull request under review. (This includes rebasing your branch onto the updated base branch. Use a merge operation, instead or hit the "Update branch" button at the bottom of the Github PR page.) This preserves the ability for reviewers to filter changes since their last review.
A pull request must obtain approvals from at least two reviewers before it can be considered for merge by a Maintainer. Maintainers retain discretion to require more approvals if they feel the credibility of the existing approvals is insufficient.
Pull requests must be merged by squash-and-merge to preserve a linear history for the develop branch.
In addition to those guidelines, please start your PR title with one of the following:
build: - The changes only affect the build process, including CMake and/or Conan settings.feat: New feature (change which adds functionality).fix: - The primary purpose is to fix an existing bug.docs: - The changes only affect documentation.test: - The changes only affect unit tests.ci: Continuous Integration (changes to our CI configuration files and scripts).style: Code style (formatting).refactor: - The changes refactor code without affecting functionality.perf: - The primary purpose is performance improvements.chore: - Other tasks that don't affect the binary, but don't fit any of the other cases. e.g. git settings, clang-tidy, removing dead code, dropping support for older tooling.First letter after the type prefix should be capitalized, and the type prefix should be followed by a colon and a space. e.g. feat: Add support for Borrowing Protocol.
A pull request should only have the "Ready to merge" label added when it meets a few criteria:
develop). This is usually accomplished by merging the base branch into the feature branch, but if the other criteria are met, the changes can be squashed and rebased on top of the base branch.Once the "Ready to merge" label is added, a maintainer may merge the PR at any time, so don't use it lightly.
This is a non-exhaustive list of recommended style guidelines. These are not always strictly enforced and serve as a way to keep the codebase coherent rather than a set of thou shalt not commandments.
All code must conform to clang-format version 21, according to the settings in .clang-format, unless the result would be unreasonably difficult to read or maintain. To demarcate lines that should be left as-is, surround them with comments like this:
You can format individual files in place by running clang-format -i <file>... from any directory within this project.
There is a Continuous Integration job that runs clang-format on pull requests. If the code doesn't comply, a patch file that corrects auto-fixable formatting issues is generated.
To download the patch file:
clang-format / check (pull_request) Failing after #s -> click Details to open the details page.Artifacts -> click clang-format.patchgit apply [patch-file-name].You can install a pre-commit hook to automatically run clang-format before every commit:
All code must pass clang-tidy checks according to the settings in .clang-tidy.
There is a Continuous Integration job that runs clang-tidy on pull requests. The CI will check:
.cpp, .h, .ipp) when only code files are modified.clang-tidy configuration file is changedThis ensures that configuration changes don't introduce new warnings across the codebase.
See the environment setup guide for platform-specific installation instructions.
Before running clang-tidy, you must build the project to generate required files (particularly protobuf headers). Refer to `BUILD.md` for build instructions.
Then run clang-tidy on your local changes:
This will check all source files in the src, include and tests directories using the compile commands from your build directory. If you wish to automatically fix whatever clang-tidy finds and is capable of fixing, add -fix to the above command:
We are using Antithesis for continuous fuzzing, and keep a copy of Antithesis C++ SDK in external/antithesis-sdk. One of the aims of fuzzing is to identify bugs by finding external conditions which cause contracts violations inside xrpld. The contracts are expressed as XRPL_ASSERT or UNREACHABLE (defined in include/xrpl/beast/utility/instrumentation.h), which are effectively (outside of Antithesis) wrappers for assert(...) with added name. The purpose of name is to provide contracts with stable identity which does not rely on line numbers.
When xrpld is built with the Antithesis instrumentation enabled (using voidstar CMake option) and ran on the Antithesis platform, the contracts become test properties; otherwise they are just like a regular assert. To learn more about Antithesis, see How Antithesis Works and C++ SDK
We continue to use the old style assert or assert(false) in certain locations, where the reporting of contract violations on the Antithesis platform is either not possible or not useful.
For this reason:
assert or assert(false) contracts should continue to be used:constexpr functionssrc/testbeast/test and beast/unit_test)assert; use XRPL_ASSERT instead, giving it unique name, with the short description of the contract.assert(false); use UNREACHABLE instead, giving it unique name, with the description of the condition being violated: and a brief (typically at most five words) description. UNREACHABLE contracts can use slightly longer descriptions. If there are multiple overloads of the function, use common sense to balance both brevity and unambiguity of the function name. NOTE: the purpose of name is to provide stable means of unique identification of every contract; for this reason try to avoid elements which can change in some obvious refactors or when reinforcing the condition.UNREACHABLE) should describe the expected condition, as in "I assert that _expected_ is true".UNREACHABLE should describe the unexpected situation which caused the line to have been reached.UNREACHABLE macro "Json::operator==(Value, Value) : invalid type"; example good name for an XRPL_ASSERT macro "Json::Value::asCString : valid type"."RFC1751::insert(char* s, int x, int start, int length) : length is greater than or equal zero" (missing namespace, unnecessary full function signature, description too verbose). Good name: "xrpl::RFC1751::insert : minimum length".contract.cpp)std::unreachableTo execute all unit tests:
xrpld --unittest --unittest-jobs=<number of cores>
(Note: Using multiple cores on a Mac M1 can cause spurious test failures. The cause is still under investigation. If you observe this problem, try specifying fewer jobs.)
To run a specific set of test suites:
Note: In this example, all tests with prefix TestSuiteName will be run, so if TestSuiteName1 and TestSuiteName2 both exist, then both tests will run. Alternatively, if the unit test name finds an exact match, it will stop doing partial matches, i.e. if a unit test with a title of TestSuiteName exists, then no other unit test will be executed, apart from TestSuiteName.
Maintainers are ecosystem participants with elevated access to the repository. They are able to push new code, make decisions on when a release should be made, etc.
New maintainers can be proposed by two existing maintainers, subject to a vote by a quorum of the existing maintainers. A minimum of 50% support and a 50% participation is required. In the event of a tie vote, the addition of the new maintainer will be rejected.
Existing maintainers can resign, or be subject to a vote for removal at the behest of two existing maintainers. A minimum of 60% agreement and 50% participation are required. The XRP Ledger Foundation will have the ability, for cause, to remove an existing maintainer without a vote.
Maintainers are users with maintain or admin access to the repo.
Code Reviewers are developers who have the ability to review, approve, and in some cases merge source code changes.
Developers not on this list are able and encouraged to submit feedback on pending code changes (open pull requests).
These instructions assume you have your git upstream remotes configured to avoid accidental pushes to the main repo, and a remote group specifying both of them. e.g.
You can use the setup-upstreams script to set this up.
It also assumes you have a default gpg signing key set up in git. e.g.
The maintainer should double-check that the PR has met all the necessary criteria, and can request additional information from the owner, or additional reviews, and can always feel free to remove the "Ready to merge" label if appropriate. The maintainer has final say on whether a PR gets merged, and are encouraged to communicate and issues or concerns to other maintainers.
Most pull requests don't need special handling, and can simply be merged using the "Squash and merge" button on the Github UI. Update the suggested commit message, or modify it as needed.
Some pull requests need to be pushed to develop as more than one commit. A PR author may request to merge as separate commits. They must justify why separate commits are needed, and specify how they would like the commits to be merged. If you disagree with the author, discuss it with them directly.
If the process is reasonable, follow it. The simplest option is to do a fast forward only merge (--ff-only) on the command line and push to develop.
Some examples of when separate commits are worthwhile are:
git bisect would not be much help if it determined this PR introduced a problem.Either way, check that:
develop.git commit --amend -S to sign them yourself.The "Create a merge commit" and "Rebase and merge" options should be disabled in the Github UI, but if you ever find them available Do not use them!
All releases, including release candidates and betas, are handled differently from typical PRs. Most importantly, never use the Github UI to merge a release.
Rippled uses a linear workflow model that can be summarized as:
develop branch.develop, which is pushed to release.develop are considered stable and mature enough to be ready to release, a release candidate (RC) is built and tagged from develop, and merged to release.release, while other development continues on develop. Effectively, release is forked from develop. Changes to release must be reverse merged to develop.master.master, and then reverse merged to develop.develop branchdevelop branch will be ready to go, with all relevant PRs already merged.If there are several pending PRs, do not use the Github UI, because the delays waiting for CI in between each merge will be unnecessarily onerous. (Incidentally, this process can also be used to merge if the Github UI has issues.) Merge each PR branch directly to a release-next on your local machine and create a single PR, then push your branch to develop.
The workflow may look something like:
You can also use the squash-branches script.
You may also need to manually close the open PRs after the changes are merged to develop. Be sure to include the commit ID.
This includes, betas, and the first release candidate (RC).
release-next branch hanging around. Then make a release-next branch that only changes the version number. e.g.You can also use the update-version script. 2. Create a Pull Request for release-next with **develop** as the base branch.
develop branch using your release-next branch. Do not use the Github UI. It's important to preserve commit IDs.develop branch, push again, and go back to step 3.develop is closed. Github should do it automatically.develop can continue (other PRs may be merged).release-next with **release** as the base branch. Instead of the default template, reuse and update the message from the previous release. Include the following verbiage somewhere in the description:release.release-next branch on the repo. Use the Github UI or:Once the first release candidate is merged into release, then release and develop are allowed to diverge.
If a bug or issue is discovered in a version that has a release candidate being tested, any fix and new version will need to be applied against release, then reverse-merged to develop. This helps keep git history as linear as possible.
A release-next branch will be created from release, and any further work for that release must be based on release-next. Specifically, PRs must use release-next as the base, and those PRs will be merged directly to release-next when approved. Changes should be restricted to bug fixes, but other changes may be necessary from time to time.
release-next as the base, so they can be merged directly in to it. Unlike develop, though, release-next can be thrown away and recreated if necessary.release-next. You can use the update-version script for this, too.release-next into release.Once the RC is merged and tagged, it needs to be reverse merged into develop as soon as possible.
upstream/develop. The branch name is not important, but could include "mergeNNNrcN". E.g. For release A.B.C-rcD, use mergeABCrcD.release into your branch.BuildInfo.cpp will have a conflict with the version number. Resolve it with the version from develop - the higher version.upstream if you have permission), and open a normal PR against develop. The "High level overview" can simply indicate that this is a merge of the RC. The "Context" should summarize the changes from the RC. Include the following text prominently:develop is updated before this PR is merged, do not merge develop back into your branch. Instead rebase preserving merges, or do the merge again. (See also the rerere git config setting.)develop.Development on develop can proceed as normal.
A final release is any release that is not a beta or RC, such as 2.2.0.
Only code that has already been tested and vetted across all three platforms should be included in a final release. Most of the time, that means that the commit immediately preceding the commit setting the version number will be an RC. Occasionally, there may be last-minute bug fixes included as well. If so, those bug fixes must have been tested internally as if they were RCs (at minimum, ensuring unit tests pass, and the app starts, syncs, and stops cleanly across all three platforms.)
If in doubt, make an RC first.
The process for building a final release is very similar to the process for building a beta, except the code will be moving from release to master instead of from develop to release, and both branches will be pushed at the same time.
master-next branch hanging around. Then make a master-next branch that only changes the version number. As above, or using the update-version script.master-next with **master** as the base branch. Instead of the default template, reuse and update the message from the previous final release. Include the following verbiage somewhere in the description:master-next, and move development back to release, issuing more RCs as necessaryrelease and master.master-next branch on the repo. Use the Github UI or:API-VERSION-[n].md with the changes for this release (if any are missing).On occasion, a bug or issue is discovered in a version that already had a final release. Most of the time, development will have started on the next version, and will usually have changes in develop and often in release.
Because git history is kept as linear as possible, any fix and new version will need to be applied against master.
The process for building a hotfix release is very similar to [the process for building release candidates after the first](#release-candidates-after-the-first) and for building a final release, except the changes will be done against master instead of release.
If there is only a single issue for the hotfix, the work can be done in any branch. When it's ready to merge, jump to step 3 using your branch instead of master-next.
master-next branch from master.master-next as the base, so they can be merged directly in to it. Unlike develop, though, master-next can be thrown away and recreated if necessary.master-next with **master** as the base branch. Instead of the default template, reuse and update the message from the previous final release. Include the following verbiage somewhere in the description:master-next as needed, but ensure that the changes are properly squashed, and the version setting commit remains lastmaster only.master-next branch on the repo.Once the hotfix is released, it needs to be reverse merged into develop as soon as possible. It may also need to be merged into release if a release candidate is under development.
upstream/develop. The branch name is not important, but could include "mergeNNN". E.g. For release 2.2.3, use merge223.BuildInfo.cpp will have a conflict with the version number. Resolve it with the version from develop - the higher version.develop. The "High level overview" can simply indicate that this is a merge of the hotfix version. The "Context" should summarize the changes from the hotfix. Include the following text prominently:develop is updated before this PR is merged, do not merge develop back into your branch. Instead rebase preserving merges, or do the merge again. (See also the rerere git config setting.)develop.Development on develop can proceed as normal. It is recommended to create a beta (or RC) immediately to ensure that everything worked as expected.
Historically, once a final release is tagged and packages are released, versions older than the latest final release are no longer supported. However, there is a possibility that a very high severity bug may occur in a non-amendment blocked version that is still being run by a significant fraction of users, which would necessitate a hotfix / point release to that version as well as any later versions.
This scenario would follow the same basic procedure as above, except that none of develop, release, or master would be touched during the release process.
In this example, consider if version 2.1.1 needed to be patched.
upstream) repo.master-2.1.2as the base branch for any merging, packaging, etc.master-2.1.2 and master212-next. However, it may be useful to keep master-2.1.2 around indefinitely for reference.master-2.1.2 into master, release a hotfix for that version, then reverse merge from master to develop. (Please don't do this unless absolutely necessary.)