|
| 1 | +# Merge Conflict Resolution Guide |
| 2 | + |
| 3 | +## Description |
| 4 | +Use when resolving merge conflicts after merging `develop` into a feature branch. Covers the key files that commonly conflict, field number allocation, namespace conventions, and the correct resolution strategy for each file type. |
| 5 | + |
| 6 | +## General Principles |
| 7 | + |
| 8 | +### Namespace |
| 9 | +- All code uses `namespace xrpl { }` (not `ripple`). If a conflict shows `namespace ripple`, take the `xrpl` version. |
| 10 | +- Qualified references: `xrpl::sign(...)`, `xrpl::sha256_hasher`, etc. Never `ripple::`. |
| 11 | + |
| 12 | +### Header Guards |
| 13 | +- Always `#pragma once`. Never `#ifndef` include guards. If a conflict shows old-style guards, take the `#pragma once` version. |
| 14 | + |
| 15 | +### Copyright Headers |
| 16 | +- The `develop` branch has removed copyright/license comment blocks from source files. If a conflict is just about the copyright header, take `develop`'s version (no header). |
| 17 | + |
| 18 | +### File Moves |
| 19 | +- Many files have been moved from `src/xrpld/` to `src/libxrpl/` or from `src/xrpld/` headers to `include/xrpl/`. If git shows a file as "deleted in develop" but it exists at a new path, delete the old-path file (`git rm`) and keep the new-path version. |
| 20 | +- Common moves: |
| 21 | + - `src/xrpld/app/tx/detail/*.h` → `include/xrpl/tx/transactors/*.h` |
| 22 | + - `src/xrpld/app/tx/detail/*.cpp` → `src/libxrpl/tx/*.cpp` or `src/libxrpl/tx/transactors/*.cpp` |
| 23 | + - `src/xrpld/app/tx/detail/InvariantCheck.cpp` → `src/libxrpl/tx/invariants/InvariantCheck.cpp` |
| 24 | + |
| 25 | +### Transactor Migration |
| 26 | +When merging transactors from `src/xrpld/app/tx/detail/` into `develop`, they must be fully migrated to the new libxrpl structure: |
| 27 | + |
| 28 | +**File locations:** |
| 29 | +- Header: `src/xrpld/app/tx/detail/Foo.h` → `include/xrpl/tx/transactors/Foo.h` |
| 30 | +- Source: `src/xrpld/app/tx/detail/Foo.cpp` → `src/libxrpl/tx/transactors/Foo.cpp` |
| 31 | +- Delete the old `.cpp` (both `src/xrpld/` and `src/libxrpl/` are GLOB_RECURSE discovered — duplicates cause linker errors) |
| 32 | +- Optionally leave a forwarding header at the old `.h` location: `#include <xrpl/tx/transactors/Foo.h>` |
| 33 | + |
| 34 | +**Header changes:** |
| 35 | +- `#pragma once` (not `#ifndef` guards) |
| 36 | +- `namespace xrpl` (not `ripple`) |
| 37 | +- `#include <xrpl/tx/Transactor.h>` (not `<xrpld/app/tx/detail/Transactor.h>`) |
| 38 | + |
| 39 | +**Source changes:** |
| 40 | +- `namespace xrpl` (not `ripple`) |
| 41 | +- `#include <xrpl/tx/transactors/Foo.h>` (not `<xrpld/app/tx/detail/Foo.h>`) |
| 42 | +- `#include <xrpl/ledger/ApplyView.h>` (not `<xrpld/ledger/ApplyView.h>`) |
| 43 | +- `#include <xrpl/ledger/View.h>` for `describeOwnerDir` |
| 44 | + |
| 45 | +**API changes in the new Transactor:** |
| 46 | +- `preflight1()` and `preflight2()` are **private** — transactor `preflight()` must NOT call them. The framework calls them automatically. |
| 47 | +- Flag checking (`tfUniversalMask`) is handled by the framework via `getFlagsMask()`. Override `getFlagsMask()` only if custom flag handling is needed; otherwise the default handles `tfUniversalMask`. |
| 48 | +- A simple `preflight()` that only did flag checks + preflight1/preflight2 should just `return tesSUCCESS;` |
| 49 | +- `ctx_.app.journal(...)` → `ctx_.registry.journal(...)` |
| 50 | +- `ctx_.app` does not exist in the new `ApplyContext`; use `ctx_.registry` for service access |
| 51 | + |
| 52 | +**transactions.macro entry:** |
| 53 | +Every transactor must have a `#if TRANSACTION_INCLUDE` block: |
| 54 | +```cpp |
| 55 | +#if TRANSACTION_INCLUDE |
| 56 | +# include <xrpl/tx/transactors/Foo.h> |
| 57 | +#endif |
| 58 | +TRANSACTION(ttFOO, <number>, Foo, ...) |
| 59 | +``` |
| 60 | +
|
| 61 | +### Include Paths |
| 62 | +- `develop` uses the new macro-based transactor include system via `transactions.macro` with `TRANSACTION_INCLUDE`. Old-style explicit `#include <xrpld/app/tx/detail/*.h>` lists should be replaced with the macro approach. |
| 63 | +
|
| 64 | +## File-Specific Resolution Rules |
| 65 | +
|
| 66 | +### `include/xrpl/protocol/detail/features.macro` |
| 67 | +
|
| 68 | +New feature amendments go **at the top** of the active list (below the macro guard checks and `// clang-format off`), in reverse chronological order. |
| 69 | +
|
| 70 | +``` |
| 71 | +// Add new amendments to the top of this list. |
| 72 | +// Keep it sorted in reverse chronological order. |
| 73 | + |
| 74 | +XRPL_FEATURE(MyNewFeature, Supported::yes, VoteBehavior::DefaultNo) |
| 75 | +XRPL_FIX (ExistingFix, Supported::yes, VoteBehavior::DefaultNo) |
| 76 | +... |
| 77 | +``` |
| 78 | +
|
| 79 | +Resolution: Keep both sides' new amendments. Place your feature branch's new amendments at the very top. |
| 80 | +
|
| 81 | +### `include/xrpl/protocol/detail/sfields.macro` |
| 82 | +
|
| 83 | +Fields are grouped by type (UINT32, UINT64, UINT128, etc.) with common and uncommon sections. Each field has a unique **type + field number** pair. |
| 84 | +
|
| 85 | +**Resolution strategy:** |
| 86 | +1. Keep both sides' new fields. |
| 87 | +2. Check for field number collisions within each type. Use the next available number for your feature's fields. |
| 88 | +3. Common fields come first, uncommon fields after (there's a comment separator). |
| 89 | +
|
| 90 | +To find the next available field number for a type: |
| 91 | +```bash |
| 92 | +grep "TYPED_SFIELD.*UINT32" include/xrpl/protocol/detail/sfields.macro | sed 's/.*UINT32, *//;s/).*//' | sort -n |
| 93 | +``` |
| 94 | + |
| 95 | +Similarly for OBJECT and ARRAY types (using `UNTYPED_SFIELD`): |
| 96 | +```bash |
| 97 | +grep "UNTYPED_SFIELD.*OBJECT" include/xrpl/protocol/detail/sfields.macro | sed 's/.*OBJECT, *//;s/).*//' | sort -n |
| 98 | +grep "UNTYPED_SFIELD.*ARRAY" include/xrpl/protocol/detail/sfields.macro | sed 's/.*ARRAY, *//;s/).*//' | sort -n |
| 99 | +``` |
| 100 | + |
| 101 | +### `include/xrpl/protocol/detail/transactions.macro` |
| 102 | + |
| 103 | +Transaction types have a unique **transaction type number**. New feature transactions go **at the bottom** of the active transaction list (before the system transactions starting at 100+). |
| 104 | + |
| 105 | +**Resolution strategy:** |
| 106 | +1. Keep all of `develop`'s transactions. |
| 107 | +2. Add your feature's transactions at the bottom with the next available number. |
| 108 | +3. Use the new 7-argument `TRANSACTION` macro format: |
| 109 | + |
| 110 | +```cpp |
| 111 | +/** Description of the transaction */ |
| 112 | +#if TRANSACTION_INCLUDE |
| 113 | +# include <xrpl/tx/transactors/MyTransactor.h> |
| 114 | +#endif |
| 115 | +TRANSACTION(ttMY_TX, <next_number>, MyTx, |
| 116 | + Delegation::delegable, |
| 117 | + featureMyFeature, |
| 118 | + noPriv, ({ |
| 119 | + {sfSomeField, soeREQUIRED}, |
| 120 | +})) |
| 121 | +``` |
| 122 | +
|
| 123 | +To find the next available transaction number: |
| 124 | +```bash |
| 125 | +grep "^TRANSACTION(" include/xrpl/protocol/detail/transactions.macro | sed 's/.*,\s*\([0-9]*\),.*/\1/' | sort -n |
| 126 | +``` |
| 127 | + |
| 128 | +Note: Transaction numbers 100+ are reserved for system transactions (amendments, fees, UNL). |
| 129 | + |
| 130 | +### `include/xrpl/protocol/detail/ledger_entries.macro` |
| 131 | + |
| 132 | +Ledger entry types have a unique **ledger type number** (hex). New entries go **at the bottom** of the active list. |
| 133 | + |
| 134 | +**Resolution strategy:** |
| 135 | +1. Keep all of `develop`'s entries. |
| 136 | +2. Add your feature's entries at the bottom with the next available hex number. |
| 137 | +3. Check for collisions: |
| 138 | + |
| 139 | +```bash |
| 140 | +grep "^LEDGER_ENTRY(" include/xrpl/protocol/detail/ledger_entries.macro | grep -o '0x[0-9a-fA-F]*' | sort |
| 141 | +``` |
| 142 | + |
| 143 | +### `src/libxrpl/protocol/Indexes.cpp` |
| 144 | + |
| 145 | +The `LedgerNameSpace` enum assigns a unique single character to each ledger entry type for index hashing. |
| 146 | + |
| 147 | +**Resolution strategy:** |
| 148 | +1. Keep both sides' entries. |
| 149 | +2. Check for character collisions. Each entry needs a unique char. |
| 150 | +3. Find used characters: |
| 151 | + |
| 152 | +```bash |
| 153 | +grep -E "^\s+[A-Z_]+ = " src/libxrpl/protocol/Indexes.cpp | sed "s/.*= '//;s/'.*//" | sort | tr -d '\n' |
| 154 | +``` |
| 155 | + |
| 156 | +Also check the deprecated chars (reserved, cannot reuse): |
| 157 | +```bash |
| 158 | +grep "deprecated" src/libxrpl/protocol/Indexes.cpp | sed "s/.*= '//;s/'.*//" |
| 159 | +``` |
| 160 | + |
| 161 | +### `src/libxrpl/protocol/InnerObjectFormats.cpp` |
| 162 | + |
| 163 | +Inner object formats define the fields allowed inside array elements (e.g., `sfSigners`, `sfPasskeys`). |
| 164 | + |
| 165 | +**Resolution:** Keep both sides' `add(...)` calls. No numbering conflicts here — just ensure no duplicate registrations. |
| 166 | + |
| 167 | +### `src/libxrpl/protocol/STTx.cpp` |
| 168 | + |
| 169 | +The `singleSignHelper` function was refactored in `develop`: |
| 170 | +- Parameter name: `sigObject` (not `signer`) |
| 171 | +- Signature retrieval: `sigObject.getFieldVL(sfTxnSignature)` (not `getSignature(signer)`) |
| 172 | +- No `fullyCanonical` parameter — canonical sig checking was simplified |
| 173 | + |
| 174 | +**Resolution:** Take `develop`'s function signatures and patterns. Adapt any feature-specific logic to match. |
| 175 | + |
| 176 | +### `src/libxrpl/protocol/SecretKey.cpp` |
| 177 | + |
| 178 | +**Resolution:** Use `namespace xrpl`. Keep any additional `#include` directives your feature needs (e.g., OpenSSL headers for new key types). |
| 179 | + |
| 180 | +### `src/libxrpl/tx/Transactor.cpp` |
| 181 | + |
| 182 | +The `checkSign` / `checkSingleSign` / `checkMultiSign` functions were refactored in `develop`: |
| 183 | +- `checkSingleSign` no longer takes a `Rules` parameter |
| 184 | +- `checkSign` has a new overload taking `PreclaimContext` |
| 185 | +- `checkBatchSign` uses the simplified `checkSingleSign` call |
| 186 | + |
| 187 | +**Resolution:** Take `develop`'s function signatures. Add any new authentication checks (e.g., passkey verification) into `checkSingleSign` before the final `return tefBAD_AUTH`, using `view.read(...)` to check ledger state. |
| 188 | + |
| 189 | +### `src/libxrpl/tx/applySteps.cpp` |
| 190 | + |
| 191 | +**Resolution:** Always take `develop`'s macro-based include approach: |
| 192 | +```cpp |
| 193 | +#include <xrpl/tx/applySteps.h> |
| 194 | +#pragma push_macro("TRANSACTION") |
| 195 | +#undef TRANSACTION |
| 196 | + |
| 197 | +#define TRANSACTION(...) |
| 198 | +#define TRANSACTION_INCLUDE 1 |
| 199 | + |
| 200 | +#include <xrpl/protocol/detail/transactions.macro> |
| 201 | + |
| 202 | +#undef TRANSACTION |
| 203 | +#pragma pop_macro("TRANSACTION") |
| 204 | +``` |
| 205 | + |
| 206 | +Never use the old explicit `#include <xrpld/app/tx/detail/*.h>` list. |
| 207 | + |
| 208 | +### `src/test/jtx/impl/utility.cpp` |
| 209 | + |
| 210 | +The `sign` function was refactored in `develop`: |
| 211 | +- Takes `sigObject` parameter: `sign(Json::Value& jv, Account const& account, Json::Value& sigObject)` |
| 212 | +- Has an overload: `sign(Json::Value& jv, Account const& account)` that calls `sign(jv, account, jv)` |
| 213 | +- Uses `xrpl::sign(...)` not `ripple::sign(...)` |
| 214 | + |
| 215 | +**Resolution:** Take `develop`'s function signatures. Adapt feature-specific signing logic to the new pattern. |
| 216 | + |
| 217 | +## Conflict Resolution Checklist |
| 218 | + |
| 219 | +1. List all conflicted files: `git diff --name-only --diff-filter=U` |
| 220 | +2. Check which have actual conflict markers vs just unmerged: `grep -l "<<<<<<< HEAD" <file>` |
| 221 | +3. For files without markers: stage them or `git rm` if they were moved |
| 222 | +4. For each file with markers: |
| 223 | + - Take `develop`'s structural changes (namespace, function signatures, macro formats) |
| 224 | + - Preserve your feature's additions (new fields, entries, transactions, logic) |
| 225 | + - Resolve any numbering collisions by incrementing to the next available number |
| 226 | +5. Stage resolved files: `git add <file>` |
| 227 | +6. Verify no remaining markers: `grep -r "<<<<<<< " --include="*.cpp" --include="*.h" --include="*.macro"` |
| 228 | +7. Verify no remaining unmerged: `git diff --name-only --diff-filter=U` |
0 commit comments