Merge remote-tracking branch 'XRPLF/ximinez/number-division-accuracy' into ximinez/number-maxint-range

* XRPLF/ximinez/number-division-accuracy:
  CI feedback: Add test cases covering other rounding modes
  Apply suggestions from code review
  Accept AI suggestion
  Update the testUpwardRoundsDown test to run under all scales
  Review feedback from Tapanito and AI
  Skip clang-tidy false positive: misc-include-cleaner
  ci: Run PR title and description checks on staging and release branches (7331)
  style: Run shfmt on workflows, actions and markdown bash code (7333)
  Remove TMax entirely from normalizeToRange; check type matching directly
  Tweak how the denominator is handled in division
  Minor fixes: missing include, variable init, typo
  Test optimization: Number_test::pow10
  Significant rewrite
  Use the local range instead of calling a function
  Make Number::operator/= significantly more accurate
This commit is contained in:
Ed Hennis
2026-06-01 13:16:25 -04:00
23 changed files with 1075 additions and 235 deletions

View File

@@ -37,12 +37,12 @@ runs:
run: |
echo 'Installing dependencies.'
conan install \
--profile ci \
--build="${BUILD_OPTION}" \
--options:host='&:tests=True' \
--options:host='&:xrpld=True' \
--settings:all build_type="${BUILD_TYPE}" \
--conf:all tools.build:jobs=${BUILD_NPROC} \
--conf:all tools.build:verbosity="${LOG_VERBOSITY}" \
--conf:all tools.compilation:verbosity="${LOG_VERBOSITY}" \
.
--profile ci \
--build="${BUILD_OPTION}" \
--options:host='&:tests=True' \
--options:host='&:xrpld=True' \
--settings:all build_type="${BUILD_TYPE}" \
--conf:all tools.build:jobs=${BUILD_NPROC} \
--conf:all tools.build:verbosity="${LOG_VERBOSITY}" \
--conf:all tools.compilation:verbosity="${LOG_VERBOSITY}" \
.

View File

@@ -15,7 +15,7 @@ runs:
shell: bash
env:
VERSION: ${{ github.ref_name }}
run: echo "VERSION=${VERSION}" >> "${GITHUB_ENV}"
run: echo "VERSION=${VERSION}" >>"${GITHUB_ENV}"
# When a tag is not pushed, then the version (e.g. 1.2.3-b0) is extracted
# from the BuildInfo.cpp file and the shortened commit hash appended to it.
@@ -28,17 +28,17 @@ runs:
echo 'Extracting version from BuildInfo.cpp.'
VERSION="$(cat src/libxrpl/protocol/BuildInfo.cpp | grep "versionString =" | awk -F '"' '{print $2}')"
if [[ -z "${VERSION}" ]]; then
echo 'Unable to extract version from BuildInfo.cpp.'
exit 1
echo 'Unable to extract version from BuildInfo.cpp.'
exit 1
fi
echo 'Appending shortened commit hash to version.'
SHA='${{ github.sha }}'
VERSION="${VERSION}+${SHA:0:7}"
echo "VERSION=${VERSION}" >> "${GITHUB_ENV}"
echo "VERSION=${VERSION}" >>"${GITHUB_ENV}"
- name: Output version
id: version
shell: bash
run: echo "version=${VERSION}" >> "${GITHUB_OUTPUT}"
run: echo "version=${VERSION}" >>"${GITHUB_OUTPUT}"

403
.github/scripts/format-inline-bash.py vendored Executable file
View File

@@ -0,0 +1,403 @@
#!/usr/bin/env python3
"""
Format embedded shell snippets using the shfmt hook configured in
.pre-commit-config.yaml.
Two shapes are recognised:
* YAML workflow/action files: literal block-scalar runs (`run: |`) and
single-line runs (`run: some command`). A single-line run is upgraded to
a `run: |` block scalar if shfmt's output spans multiple lines.
* Markdown files: ``` ```bash ``` fenced code blocks.
Any block that shfmt cannot parse is skipped with a warning on stderr, so
the file is left untouched and surrounding blocks still get formatted.
For each occurrence the body is dedented, written to a temp .sh file,
formatted via `pre-commit run shfmt --files <temp>` (falling back to
`prek`), then re-indented and written back in place.
When invoked without arguments, every .yml/.yaml under .github/ plus every
.md file in the repo is scanned. When invoked with file arguments (the
pre-commit case), only those files are processed.
"""
from __future__ import annotations
import re
import shutil
import subprocess
import sys
import tempfile
from dataclasses import dataclass
from pathlib import Path
from typing import Union
REPO = Path(__file__).resolve().parents[2]
_HOOK_RUNNER = next((cmd for cmd in ("pre-commit", "prek") if shutil.which(cmd)), None)
if _HOOK_RUNNER is None:
sys.exit("error: neither `pre-commit` nor `prek` found on PATH")
RUN_BLOCK_RE = re.compile(r"^(?P<prefix>[ \t]*(?:- )?)run:[ \t]*\|[+-]?[ \t]*$")
RUN_INLINE_RE = re.compile(
r"^(?P<prefix>[ \t]*(?:- )?)run:[ \t]+" r"(?P<value>(?!\|[+-]?[ \t]*$)\S.*?)[ \t]*$"
)
MD_BASH_OPEN_RE = re.compile(r"^(?P<indent>[ ]{0,3})`{3}bash[ \t]*$")
MD_FENCE_CLOSE_RE = re.compile(r"^[ ]{0,3}`{3,}[ \t]*$")
@dataclass(frozen=True)
class BlockRun:
"""A `run: |` block scalar; `body_start:body_end` slices into `lines`."""
body_start: int
body_end: int
body_indent: int
@dataclass(frozen=True)
class InlineRun:
"""A single-line `run: value` at `line_idx`."""
line_idx: int
prefix: str
value: str
@dataclass(frozen=True)
class MdBashBlock:
"""A markdown ``` ```bash ``` fenced code block.
`body_start:body_end` slices into the file's lines; `open_line_idx`
points at the opening fence line.
"""
open_line_idx: int
body_start: int
body_end: int
body_indent: int
RunItem = Union[BlockRun, InlineRun]
def _scan_block_body(
lines: list[str], body_start: int, run_col: int
) -> tuple[int | None, int]:
"""Locate the body of a `run: |` block scalar starting at `body_start`.
Returns `(body_indent, scan_end)`. `scan_end` is the line index where the
outer scanner should resume. `body_indent` is `None` when no body is
present (the scalar is empty, or the next non-blank line has indent
`<= run_col`).
"""
body_indent: int | None = None
scan_end = len(lines)
for idx in range(body_start, len(lines)):
line = lines[idx]
if line.strip() == "":
continue
indent = len(line) - len(line.lstrip(" "))
if body_indent is None:
if indent > run_col:
body_indent = indent
else:
scan_end = idx
break
elif indent < body_indent:
scan_end = idx
break
if body_indent is not None:
while scan_end > body_start and lines[scan_end - 1].strip() == "":
scan_end -= 1
if scan_end <= body_start:
body_indent = None
return body_indent, scan_end
def find_run_blocks(lines: list[str]) -> list[RunItem]:
"""Return run items in document order."""
items: list[RunItem] = []
line_idx = 0
while line_idx < len(lines):
line = lines[line_idx]
if block_match := RUN_BLOCK_RE.match(line):
run_col = len(block_match.group("prefix"))
body_start = line_idx + 1
body_indent, scan_end = _scan_block_body(lines, body_start, run_col)
if body_indent is not None:
items.append(
BlockRun(
body_start=body_start,
body_end=scan_end,
body_indent=body_indent,
)
)
line_idx = scan_end
continue
if inline_match := RUN_INLINE_RE.match(line):
items.append(
InlineRun(
line_idx=line_idx,
prefix=inline_match.group("prefix"),
value=inline_match.group("value"),
)
)
line_idx += 1
return items
def find_md_bash_blocks(lines: list[str]) -> list[MdBashBlock]:
"""Return ``` ```bash ``` fenced code blocks in document order."""
blocks: list[MdBashBlock] = []
line_idx = 0
while line_idx < len(lines):
open_match = MD_BASH_OPEN_RE.match(lines[line_idx])
if not open_match:
line_idx += 1
continue
body_start = line_idx + 1
close_idx = next(
(
j
for j in range(body_start, len(lines))
if MD_FENCE_CLOSE_RE.match(lines[j])
),
None,
)
if close_idx is None:
line_idx = body_start
continue
body = lines[body_start:close_idx]
non_blank = [b for b in body if b.strip()]
body_indent = (
min(len(b) - len(b.lstrip(" ")) for b in non_blank)
if non_blank
else len(open_match.group("indent"))
)
blocks.append(
MdBashBlock(
open_line_idx=line_idx,
body_start=body_start,
body_end=close_idx,
body_indent=body_indent,
)
)
line_idx = close_idx + 1
return blocks
def dedent(lines: list[str], n: int) -> list[str]:
pad = " " * n
return [
(
""
if line.strip() == ""
else (line[n:] if line.startswith(pad) else line.lstrip(" "))
)
for line in lines
]
def reindent(lines: list[str], n: int) -> list[str]:
pad = " " * n
return [pad + line if line else "" for line in lines]
_SHFMT_ERR_RE = re.compile(r"\.sh:\d+:\d+:\s")
_GHA_EXPR_RE = re.compile(r"\$\{\{.*?\}\}", re.DOTALL)
_GHA_PLACEHOLDER_RE = re.compile(r"__GHA_EXPR_(\d+)__")
def _encode_gha_exprs(text: str) -> tuple[str, list[str]]:
"""Replace `${{ ... }}` expressions with bash-safe placeholder identifiers."""
exprs: list[str] = []
def repl(match: re.Match[str]) -> str:
exprs.append(match.group(0))
return f"__GHA_EXPR_{len(exprs) - 1}__"
return _GHA_EXPR_RE.sub(repl, text), exprs
def _decode_gha_exprs(text: str, exprs: list[str]) -> str:
"""Restore `${{ ... }}` expressions from placeholder identifiers."""
return _GHA_PLACEHOLDER_RE.sub(lambda m: exprs[int(m.group(1))], text)
def shfmt_via_hook(tmp_path: Path) -> tuple[bool, str]:
# `${{ ... }}` is not valid shell, so swap it for a placeholder identifier
# that shfmt can parse, then restore it after formatting.
encoded, exprs = _encode_gha_exprs(tmp_path.read_text())
if exprs:
tmp_path.write_text(encoded)
res = subprocess.run(
[_HOOK_RUNNER, "run", "shfmt", "--files", str(tmp_path)],
cwd=REPO,
capture_output=True,
text=True,
)
output = res.stdout + res.stderr
# shfmt emits parse errors as "<path>:<line>:<col>: <message>".
parse_err = bool(_SHFMT_ERR_RE.search(output))
# A non-zero exit that is neither a parse error nor pre-commit's "I had
# to modify files" signal means the hook itself failed to run (missing
# binary, install failure, bad config, ...). Surface that loudly rather
# than silently treating it as a no-op.
if (
res.returncode != 0
and not parse_err
and "files were modified by this hook" not in output
):
sys.exit(
f"error: `{_HOOK_RUNNER} run shfmt` failed with exit {res.returncode}:\n{output}"
)
if exprs and not parse_err:
tmp_path.write_text(_decode_gha_exprs(tmp_path.read_text(), exprs))
return not parse_err, output
def _skip(path: Path, where: int, kind: str, output: str) -> None:
print(
f" shfmt could not parse {kind} at {path}:{where + 1} — skipped",
file=sys.stderr,
)
print(f" {output.strip()}", file=sys.stderr)
def process_yaml_file(path: Path, tmp_path: Path) -> int:
text = path.read_text()
had_nl = text.endswith("\n")
lines = text.split("\n")
if had_nl:
lines = lines[:-1]
items = find_run_blocks(lines)
if not items:
return 0
changed = 0
# Process in reverse so earlier indices remain valid as we splice.
for item in reversed(items):
if isinstance(item, BlockRun):
body = lines[item.body_start : item.body_end]
tmp_path.write_text("\n".join(dedent(body, item.body_indent)) + "\n")
ok, output = shfmt_via_hook(tmp_path)
if not ok:
_skip(path, item.body_start, "block", output)
continue
formatted = tmp_path.read_text().rstrip("\n")
new_body = reindent(formatted.split("\n"), item.body_indent)
if new_body != body:
lines[item.body_start : item.body_end] = new_body
changed += 1
else:
tmp_path.write_text(item.value + "\n")
ok, output = shfmt_via_hook(tmp_path)
if not ok:
_skip(path, item.line_idx, "inline run", output)
continue
formatted = tmp_path.read_text().rstrip("\n")
if formatted == item.value:
continue
formatted_lines = formatted.split("\n")
if len(formatted_lines) == 1:
lines[item.line_idx] = f"{item.prefix}run: {formatted}"
else:
body_indent = len(item.prefix) + 2
lines[item.line_idx : item.line_idx + 1] = [
f"{item.prefix}run: |",
*reindent(formatted_lines, body_indent),
]
changed += 1
new_text = "\n".join(lines) + ("\n" if had_nl else "")
if new_text != text:
path.write_text(new_text)
return changed
def process_md_file(path: Path, tmp_path: Path) -> int:
text = path.read_text()
had_nl = text.endswith("\n")
lines = text.split("\n")
if had_nl:
lines = lines[:-1]
blocks = find_md_bash_blocks(lines)
if not blocks:
return 0
changed = 0
for block in reversed(blocks):
body = lines[block.body_start : block.body_end]
tmp_path.write_text("\n".join(dedent(body, block.body_indent)) + "\n")
ok, output = shfmt_via_hook(tmp_path)
if not ok:
_skip(path, block.open_line_idx, "```bash block", output)
continue
formatted = tmp_path.read_text().rstrip("\n")
formatted_lines = formatted.split("\n") if formatted else []
new_body = reindent(formatted_lines, block.body_indent)
if new_body != body:
lines[block.body_start : block.body_end] = new_body
changed += 1
new_text = "\n".join(lines) + ("\n" if had_nl else "")
if new_text != text:
path.write_text(new_text)
return changed
def process_file(path: Path, tmp_path: Path) -> int:
if path.suffix in (".yml", ".yaml"):
return process_yaml_file(path, tmp_path)
if path.suffix == ".md":
return process_md_file(path, tmp_path)
return 0
def gather_files(argv: list[str]) -> list[Path]:
"""Return YAML workflow/action files and markdown files that we should
process — either the paths in `argv` or, when `argv` is empty, every
such file in the repo (skipping `external/`)."""
if argv:
candidates: list[Path] = [
(REPO / a).resolve() if not Path(a).is_absolute() else Path(a) for a in argv
]
else:
gh = REPO / ".github"
candidates = [
*gh.rglob("*.yml"),
*gh.rglob("*.yaml"),
*(
p
for p in REPO.rglob("*.md")
if "external" not in p.relative_to(REPO).parts
),
]
return sorted(
p
for p in candidates
if p.exists()
and (
(p.suffix in (".yml", ".yaml") and ".github" in p.parts)
or p.suffix == ".md"
)
)
def main(argv: list[str]) -> int:
files = gather_files(argv)
if not files:
return 0
with tempfile.TemporaryDirectory(prefix="format-inline-bash-") as tmpdir:
tmp_path = Path(tmpdir) / "shfmt.sh"
total = 0
for f in files:
n = process_file(f, tmp_path)
if n:
print(f"{f.relative_to(REPO)}: reformatted {n} block(s)")
total += n
return 1 if total else 0
if __name__ == "__main__":
sys.exit(main(sys.argv[1:]))

View File

@@ -100,8 +100,8 @@ jobs:
- name: Create multi-arch manifests
run: |
for tag in $(jq -cr '.tags[]' <<< "$DOCKER_METADATA_OUTPUT_JSON"); do
docker buildx imagetools create -t "$tag" "${tag}-amd64" "${tag}-arm64"
for tag in $(jq -cr '.tags[]' <<<"$DOCKER_METADATA_OUTPUT_JSON"); do
docker buildx imagetools create -t "$tag" "${tag}-amd64" "${tag}-arm64"
done
- name: Inspect image

View File

@@ -5,8 +5,17 @@ on:
types:
- checks_requested
pull_request:
types: [opened, edited, reopened, synchronize, ready_for_review]
branches: [develop]
types:
- opened
- edited
- reopened
- synchronize
- ready_for_review
branches:
- develop
- "release-*"
- "release/*"
- "staging/*"
jobs:
check_description:
@@ -20,11 +29,11 @@ jobs:
env:
PR_BODY: ${{ github.event.pull_request.body }}
if: ${{ github.event_name == 'pull_request' }}
run: printenv PR_BODY > pr_body.md
run: printenv PR_BODY >pr_body.md
- name: Check PR description differs from template
if: ${{ github.event_name == 'pull_request' }}
run: >
python .github/scripts/check-pr-description.py
--template-file .github/pull_request_template.md
--pr-body-file pr_body.md
run: |
python .github/scripts/check-pr-description.py \
--template-file .github/pull_request_template.md \
--pr-body-file pr_body.md

View File

@@ -5,10 +5,19 @@ on:
types:
- checks_requested
pull_request:
types: [opened, edited, reopened, synchronize, ready_for_review]
branches: [develop]
types:
- opened
- edited
- reopened
- synchronize
- ready_for_review
branches:
- develop
- "release-*"
- "release/*"
- "staging/*"
jobs:
check_title:
if: ${{ github.event.pull_request.draft != true }}
uses: XRPLF/actions/.github/workflows/check-pr-title.yml@291206777251b4d493641b5afbdf7c23009d2988
uses: XRPLF/actions/.github/workflows/check-pr-title.yml@cba1f0891650baf1a9c88624dc2d72573be2eb81

View File

@@ -98,7 +98,7 @@ jobs:
READY: ${{ contains(github.event.pull_request.labels.*.name, 'Ready to merge') }}
MERGE: ${{ github.event_name == 'merge_group' }}
run: |
echo "go=${{ (env.DRAFT != 'true' && env.READY == 'true') || env.FILES == 'true' || env.MERGE == 'true' }}" >> "${GITHUB_OUTPUT}"
echo "go=${{ (env.DRAFT != 'true' && env.READY == 'true') || env.FILES == 'true' || env.MERGE == 'true' }}" >>"${GITHUB_OUTPUT}"
cat "${GITHUB_OUTPUT}"
outputs:
go: ${{ steps.go.outputs.go == 'true' }}
@@ -168,9 +168,9 @@ jobs:
PR_URL: ${{ github.event.pull_request.html_url }}
run: |
gh api --method POST -H "Accept: application/vnd.github+json" -H "X-GitHub-Api-Version: 2022-11-28" \
/repos/xrplf/clio/dispatches -f "event_type=check_libxrpl" \
-F "client_payload[ref]=${{ needs.upload-recipe.outputs.recipe_ref }}" \
-F "client_payload[pr_url]=${PR_URL}"
/repos/xrplf/clio/dispatches -f "event_type=check_libxrpl" \
-F "client_payload[ref]=${{ needs.upload-recipe.outputs.recipe_ref }}" \
-F "client_payload[pr_url]=${PR_URL}"
passed:
if: failure() || cancelled()

View File

@@ -14,7 +14,7 @@ on:
jobs:
# Call the workflow in the XRPLF/actions repo that runs the pre-commit hooks.
run-hooks:
uses: XRPLF/actions/.github/workflows/pre-commit.yml@5e942d61bf32f7557a7c159cfac4712a687b3e3a
uses: XRPLF/actions/.github/workflows/pre-commit.yml@cba1f0891650baf1a9c88624dc2d72573be2eb81
with:
runs_on: ubuntu-latest
container: '{ "image": "ghcr.io/xrplf/ci/tools-rippled-pre-commit:sha-41ec7c1" }'

View File

@@ -53,7 +53,7 @@ jobs:
env:
PLATFORM: ${{ inputs.platform }}
run: |
echo "arch=${PLATFORM##*/}" >> $GITHUB_OUTPUT
echo "arch=${PLATFORM##*/}" >>$GITHUB_OUTPUT
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # v4.1.0

View File

@@ -113,7 +113,7 @@ jobs:
- name: Set ccache log file
if: ${{ inputs.ccache_enabled && runner.debug == '1' }}
run: echo "CCACHE_LOGFILE=${{ runner.temp }}/ccache.log" >> "${GITHUB_ENV}"
run: echo "CCACHE_LOGFILE=${{ runner.temp }}/ccache.log" >>"${GITHUB_ENV}"
- name: Print build environment
uses: XRPLF/actions/print-build-env@59dec886e4afb05a1724443af08baccbc045b574
@@ -146,11 +146,11 @@ jobs:
CMAKE_ARGS: ${{ inputs.cmake_args }}
run: |
cmake \
-G '${{ runner.os == 'Windows' && 'Visual Studio 17 2022' || 'Ninja' }}' \
-DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
-DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \
${CMAKE_ARGS} \
..
-G '${{ runner.os == 'Windows' && 'Visual Studio 17 2022' || 'Ninja' }}' \
-DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
-DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \
${CMAKE_ARGS} \
..
- name: Check protocol autogen files are up-to-date
working-directory: ${{ env.BUILD_DIR }}
@@ -172,10 +172,10 @@ jobs:
cmake --build . --target code_gen
DIFF=$(git -C .. status --porcelain -- include/xrpl/protocol_autogen src/tests/libxrpl/protocol_autogen)
if [ -n "${DIFF}" ]; then
echo "::error::Generated protocol files are out of date"
git -C .. diff -- include/xrpl/protocol_autogen src/tests/libxrpl/protocol_autogen
echo "${MESSAGE}"
exit 1
echo "::error::Generated protocol files are out of date"
git -C .. diff -- include/xrpl/protocol_autogen src/tests/libxrpl/protocol_autogen
echo "${MESSAGE}"
exit 1
fi
- name: Build the binary
@@ -186,18 +186,18 @@ jobs:
CMAKE_TARGET: ${{ inputs.cmake_target }}
run: |
cmake \
--build . \
--config "${BUILD_TYPE}" \
--parallel "${BUILD_NPROC}" \
--target "${CMAKE_TARGET}"
--build . \
--config "${BUILD_TYPE}" \
--parallel "${BUILD_NPROC}" \
--target "${CMAKE_TARGET}"
- name: Show ccache statistics
if: ${{ inputs.ccache_enabled }}
run: |
ccache --show-stats -vv
if [ '${{ runner.debug }}' = '1' ]; then
cat "${CCACHE_LOGFILE}"
curl ${CCACHE_REMOTE_STORAGE%|*}/status || true
cat "${CCACHE_LOGFILE}"
curl ${CCACHE_REMOTE_STORAGE%|*}/status || true
fi
- name: Upload the binary (Linux)
@@ -214,7 +214,7 @@ jobs:
working-directory: ${{ env.BUILD_DIR }}
run: |
set -o pipefail
./xrpld --definitions | python3 -m json.tool > server_definitions.json
./xrpld --definitions | python3 -m json.tool >server_definitions.json
- name: Upload server definitions
if: ${{ github.event.repository.visibility == 'public' && inputs.config_name == 'debian-bookworm-gcc-13-amd64-release' }}
@@ -231,10 +231,10 @@ jobs:
run: |
ldd ./xrpld
if [ "$(ldd ./xrpld | grep -E '(libstdc\+\+|libgcc)' | wc -l)" -eq 0 ]; then
echo 'The binary is statically linked.'
echo 'The binary is statically linked.'
else
echo 'The binary is dynamically linked.'
exit 1
echo 'The binary is dynamically linked.'
exit 1
fi
- name: Verify presence of instrumentation (Linux)
@@ -250,12 +250,12 @@ jobs:
run: |
ASAN_OPTS="include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-asan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp"
if [[ "${CONFIG_NAME}" == *gcc* ]]; then
ASAN_OPTS="${ASAN_OPTS}:alloc_dealloc_mismatch=0"
ASAN_OPTS="${ASAN_OPTS}:alloc_dealloc_mismatch=0"
fi
echo "ASAN_OPTIONS=${ASAN_OPTS}" >> ${GITHUB_ENV}
echo "TSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-tsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp" >> ${GITHUB_ENV}
echo "UBSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-ubsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp" >> ${GITHUB_ENV}
echo "LSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-lsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp" >> ${GITHUB_ENV}
echo "ASAN_OPTIONS=${ASAN_OPTS}" >>${GITHUB_ENV}
echo "TSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-tsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp" >>${GITHUB_ENV}
echo "UBSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-ubsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp" >>${GITHUB_ENV}
echo "LSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-lsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp" >>${GITHUB_ENV}
- name: Run the separate tests
if: ${{ !inputs.build_only }}
@@ -266,9 +266,9 @@ jobs:
PARALLELISM: ${{ runner.os == 'Windows' && '1' || steps.nproc.outputs.nproc }}
run: |
ctest \
--output-on-failure \
-C "${BUILD_TYPE}" \
-j "${PARALLELISM}"
--output-on-failure \
-C "${BUILD_TYPE}" \
-j "${PARALLELISM}"
- name: Run the embedded tests
if: ${{ !inputs.build_only }}
@@ -278,7 +278,7 @@ jobs:
run: |
set -o pipefail
# Coverage builds are slower due to instrumentation; use fewer parallel jobs to avoid flakiness
[ "$COVERAGE_ENABLED" = "true" ] && BUILD_NPROC=$(( BUILD_NPROC - 2 ))
[ "$COVERAGE_ENABLED" = "true" ] && BUILD_NPROC=$((BUILD_NPROC - 2))
./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 | tee unittest.log
- name: Show test failure summary
@@ -287,19 +287,19 @@ jobs:
WORKING_DIR: ${{ runner.os == 'Windows' && format('{0}\{1}', env.BUILD_DIR, inputs.build_type) || env.BUILD_DIR }}
run: |
if [ ! -d "${WORKING_DIR}" ]; then
echo "Working directory '${WORKING_DIR}' does not exist."
exit 0
echo "Working directory '${WORKING_DIR}' does not exist."
exit 0
fi
cd "${WORKING_DIR}"
if [ ! -f unittest.log ]; then
echo "unittest.log not found; embedded tests may not have run."
exit 0
echo "unittest.log not found; embedded tests may not have run."
exit 0
fi
if ! grep -E "failed" unittest.log; then
echo "Log present but no failure lines found in unittest.log."
echo "Log present but no failure lines found in unittest.log."
fi
- name: Debug failure (Linux)
if: ${{ failure() && runner.os == 'Linux' && !inputs.build_only }}
@@ -317,10 +317,10 @@ jobs:
BUILD_TYPE: ${{ inputs.build_type }}
run: |
cmake \
--build . \
--config "${BUILD_TYPE}" \
--parallel "${BUILD_NPROC}" \
--target coverage
--build . \
--config "${BUILD_TYPE}" \
--parallel "${BUILD_NPROC}" \
--target coverage
- name: Upload coverage report
if: ${{ github.repository == 'XRPLF/rippled' && !inputs.build_only && env.COVERAGE_ENABLED == 'true' }}

View File

@@ -38,9 +38,9 @@ jobs:
run: |
DIFF=$(git status --porcelain)
if [ -n "${DIFF}" ]; then
# Print the differences to give the contributor a hint about what to
# expect when running levelization on their own machine.
git diff
echo "${MESSAGE}"
exit 1
# Print the differences to give the contributor a hint about what to
# expect when running levelization on their own machine.
git diff
echo "${MESSAGE}"
exit 1
fi

View File

@@ -48,9 +48,9 @@ jobs:
run: |
DIFF=$(git status --porcelain)
if [ -n "${DIFF}" ]; then
# Print the differences to give the contributor a hint about what to
# expect when running the renaming scripts on their own machine.
git diff
echo "${MESSAGE}"
exit 1
# Print the differences to give the contributor a hint about what to
# expect when running the renaming scripts on their own machine.
git diff
echo "${MESSAGE}"
exit 1
fi

View File

@@ -70,13 +70,13 @@ jobs:
working-directory: ${{ env.BUILD_DIR }}
run: |
cmake \
-G 'Ninja' \
-DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
-DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \
-Dtests=ON \
-Dwerr=ON \
-Dxrpld=ON \
..
-G 'Ninja' \
-DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
-DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \
-Dtests=ON \
-Dwerr=ON \
-Dxrpld=ON \
..
# clang-tidy needs headers generated from proto files
- name: Build libxrpl.libpb
@@ -133,7 +133,7 @@ jobs:
- name: Write issue header
if: ${{ steps.run_clang_tidy.outcome != 'success' }}
run: |
cat > "${ISSUE_FILE}" <<EOF
cat >"${ISSUE_FILE}" <<EOF
## Clang-tidy Check Failed
### Clang-tidy Output:
@@ -144,30 +144,30 @@ jobs:
if: ${{ steps.run_clang_tidy.outcome != 'success' }}
run: |
if [ -f "${OUTPUT_FILE}" ]; then
# Extract lines containing 'error:', 'warning:', or 'note:'
grep -E '(error:|warning:|note:)' "${OUTPUT_FILE}" > filtered-output.txt || true
# Extract lines containing 'error:', 'warning:', or 'note:'
grep -E '(error:|warning:|note:)' "${OUTPUT_FILE}" >filtered-output.txt || true
# If filtered output is empty, use original (might be a different error format)
if [ ! -s filtered-output.txt ]; then
cp "${OUTPUT_FILE}" filtered-output.txt
fi
# If filtered output is empty, use original (might be a different error format)
if [ ! -s filtered-output.txt ]; then
cp "${OUTPUT_FILE}" filtered-output.txt
fi
# Truncate if too large
head -c 60000 filtered-output.txt >> "${ISSUE_FILE}"
if [ "$(wc -c < filtered-output.txt)" -gt 60000 ]; then
echo "" >> "${ISSUE_FILE}"
echo "... (output truncated, see artifacts for full output)" >> "${ISSUE_FILE}"
fi
# Truncate if too large
head -c 60000 filtered-output.txt >>"${ISSUE_FILE}"
if [ "$(wc -c <filtered-output.txt)" -gt 60000 ]; then
echo "" >>"${ISSUE_FILE}"
echo "... (output truncated, see artifacts for full output)" >>"${ISSUE_FILE}"
fi
rm filtered-output.txt
rm filtered-output.txt
else
echo "No output file found" >> "${ISSUE_FILE}"
echo "No output file found" >>"${ISSUE_FILE}"
fi
- name: Append issue footer
if: ${{ steps.run_clang_tidy.outcome != 'success' }}
run: |
cat >> "${ISSUE_FILE}" <<EOF
cat >>"${ISSUE_FILE}" <<EOF
\`\`\`
---
@@ -176,7 +176,7 @@ jobs:
- name: Create issue
if: ${{ steps.run_clang_tidy.outcome != 'success' && inputs.create_issue_on_failure }}
uses: XRPLF/actions/create-issue@36d450d12d301e8410c1b7936e5de70c291cbe36
uses: XRPLF/actions/create-issue@2b8bc36af85b88bca0dd7bfac2e2dc05f94ad712
with:
title: "Clang-tidy check failed"
body_file: ${{ env.ISSUE_FILE }}

View File

@@ -39,7 +39,7 @@ jobs:
id: generate
working-directory: .github/scripts/strategy-matrix
run: |
./generate.py --packaging --config=linux.json >> "${GITHUB_OUTPUT}"
./generate.py --packaging --config=linux.json >>"${GITHUB_OUTPUT}"
generate-version:
runs-on: ubuntu-latest

View File

@@ -42,4 +42,4 @@ jobs:
env:
GENERATE_CONFIG: ${{ inputs.os != '' && format('--config={0}.json', inputs.os) || '' }}
GENERATE_OPTION: ${{ inputs.strategy_matrix == 'all' && '--all' || '' }}
run: ./generate.py ${GENERATE_OPTION} ${GENERATE_CONFIG} >> "${GITHUB_OUTPUT}"
run: ./generate.py ${GENERATE_OPTION} ${GENERATE_CONFIG} >>"${GITHUB_OUTPUT}"

View File

@@ -66,6 +66,19 @@ repos:
- id: shfmt
args: [--write, --indent=4, --case-indent=true]
- repo: local
hooks:
- id: format-inline-bash-workflows
name: "format `run:` blocks in workflows/actions"
entry: ./.github/scripts/format-inline-bash.py
language: python
files: ^\.github/(workflows|actions)/.*\.ya?ml$
- id: format-inline-bash-markdown
name: "format ```bash blocks in markdown"
entry: ./.github/scripts/format-inline-bash.py
language: python
files: \.md$
- repo: https://github.com/streetsidesoftware/cspell-cli
rev: 4643f154907327ee0a2c7038f0296e0dd77d9776 # frozen: v10.0.0
hooks:

View File

@@ -151,8 +151,8 @@ git init
git remote add origin git@github.com:XRPLF/conan-center-index.git
git sparse-checkout init
for recipe in "${recipes[@]}"; do
echo "Checking out recipe '${recipe}'..."
git sparse-checkout add recipes/${recipe}
echo "Checking out recipe '${recipe}'..."
git sparse-checkout add recipes/${recipe}
done
git fetch origin master
git checkout master
@@ -180,7 +180,7 @@ the new recipe will be automatically pulled from the official Conan Center.
If you see an error similar to the following after running `conan profile show`:
```bash
```text
ERROR: Invalid setting '17' is not a valid 'settings.compiler.version' value.
Possible values are ['5.0', '5.1', '6.0', '6.1', '7.0', '7.3', '8.0', '8.1',
'9.0', '9.1', '10.0', '11.0', '12.0', '13', '13.0', '13.1', '14', '14.0', '15',

View File

@@ -93,6 +93,7 @@ words:
- daria
- dcmake
- dearmor
- dedented
- deleteme
- demultiplexer
- deserializaton

View File

@@ -2,6 +2,7 @@
#include <xrpl/beast/utility/instrumentation.h>
#include <array>
#include <concepts>
#include <cstdint>
#include <functional>
@@ -67,6 +68,47 @@ isPowerOfTen(T value)
return logTen(value).has_value();
}
namespace detail {
/** Builds a table of the powers of 10
*
* This function is marked consteval, so it can only be run in
* a constexpr context. This assures that it is and can only be run at
* compile time. Doing it at runtime would be pretty wasteful and
* inefficient.
*/
constexpr std::size_t kInt64Digits = 20;
consteval std::array<std::uint64_t, kInt64Digits>
buildPowersOfTen()
{
std::array<std::uint64_t, kInt64Digits> result{};
std::uint64_t power = 1;
std::size_t exponent = 0;
// end the loop early so it doesn't overflow;
for (; exponent < result.size() - 1; ++exponent, power *= 10)
{
result[exponent] = power;
if (power > std::numeric_limits<std::uint64_t>::max() / 10)
throw std::logic_error("Power of 10 table is too big");
}
result[exponent] = power;
if (power < std::numeric_limits<std::uint64_t>::max() / 10)
throw std::logic_error("Power of 10 table is not big enough for the uint64_t type");
return result;
}
} // namespace detail
constexpr std::array<std::uint64_t, detail::kInt64Digits> kPowerOfTen = detail::buildPowersOfTen();
static_assert(kPowerOfTen[0] == 1);
static_assert(kPowerOfTen[1] == 10);
static_assert(kPowerOfTen[10] == 10'000'000'000);
static_assert(
isPowerOfTen(kPowerOfTen.back()) && *logTen(kPowerOfTen.back()) == detail::kInt64Digits - 1);
/** MantissaRange defines a range for the mantissa of a normalized Number.
*
* The mantissa is in the range [min, max], where
@@ -102,6 +144,7 @@ isPowerOfTen(T value)
struct MantissaRange final
{
using rep = std::uint64_t;
enum class MantissaScale {
Small,
// LargeLegacy can be removed when fixCleanup3_2_0 is retired
@@ -115,11 +158,7 @@ struct MantissaRange final
Enabled = true,
};
explicit constexpr MantissaRange(MantissaScale scale)
: max(getMax(scale))
, internalMin(getInternalMin(scale, min))
, cuspRoundingFixEnabled(isCuspFixEnabled(scale))
, scale(scale)
explicit constexpr MantissaRange(MantissaScale sc) : scale(sc)
{
// Keep the error messages terse. Since this is constexpr, if any of these throw, it won't
// compile, so there's no real need to worry about runtime exceptions here.
@@ -132,8 +171,8 @@ struct MantissaRange final
// This is a little hacky
if ((max + 10) / 10 < min)
throw std::out_of_range("Invalid mantissa range: (max + 10) / 10 < min");
if (computeLog(internalMin) != log)
throw std::out_of_range("Invalid mantissa range: computeLog(internalMin) != log");
if (internalMin != kPowerOfTen[log])
throw std::out_of_range("Invalid mantissa range: internalMin != kPowersOfTen[log]");
}
// Explicitly delete copy and move operations
@@ -144,17 +183,17 @@ struct MantissaRange final
MantissaRange&
operator=(MantissaRange&&) = delete;
rep const max;
MantissaScale const scale;
int const log{getExponent(scale)};
rep const max{getMax(scale, log)};
rep const min{computeMin(max)};
/* Used to determine if mantissas are in range, but have fewer digits than max.
*
* Unlike min, internalMin is always an exact power of 10, so a mantissa in the internal
* representation will always have a consistent number of digits.
*/
rep const internalMin;
int const log{computeLog(min)};
CuspRoundingFix const cuspRoundingFixEnabled;
MantissaScale const scale;
rep const internalMin{getInternalMin(scale, log)};
CuspRoundingFix const cuspRoundingFixEnabled{isCuspFixEnabled(scale)};
static MantissaRange const&
getMantissaRange(MantissaScale scale);
@@ -163,20 +202,40 @@ struct MantissaRange final
getAllScales();
private:
static constexpr rep
getMax(MantissaScale scale)
static constexpr int
getExponent(MantissaScale scale)
{
switch (scale)
{
case MantissaScale::Small:
return 9'999'999'999'999'999ULL;
return 15;
case MantissaScale::LargeLegacy:
case MantissaScale::Large:
return 18;
// LCOV_EXCL_START
default:
// If called in a constexpr context, this throw assures that the build fails if an
// invalid scale is used.
throw std::runtime_error("Unknown mantissa scale");
// LCOV_EXCL_STOP
}
}
static constexpr rep
getMax(MantissaScale scale, int log)
{
switch (scale)
{
case MantissaScale::Small:
return kPowerOfTen[log + 1] - 1;
case MantissaScale::LargeLegacy:
case MantissaScale::Large:
return std::numeric_limits<std::int64_t>::max();
default:
// If called in a constexpr context, this throw assures that the build fails if an
// invalid scale is used.
throw std::runtime_error("Unknown mantissa scale"); // LCOV_EXCL_LINE
throw std::runtime_error("Unknown mantissa scale");
// LCOV_EXCL_STOP
}
}
@@ -187,25 +246,13 @@ private:
}
static constexpr rep
getInternalMin(MantissaScale scale, rep min)
getInternalMin(MantissaScale scale, int exponent)
{
switch (scale)
{
case MantissaScale::LargeLegacy:
case MantissaScale::Large:
return 1'000'000'000'000'000'000ULL;
default:
if (isPowerOfTen(min))
return min;
throw std::runtime_error("Unknown/bad mantissa scale");
}
}
static constexpr int
computeLog(rep min)
{
auto const estimate = logTenEstimate(min);
return estimate.first + (estimate.second == 1 ? 0 : 1);
if (exponent < 0 || exponent >= kPowerOfTen.size())
// If called in a constexpr context, this throw assures that the build fails if an
// invalid exponent is used.
throw std::runtime_error("Invalid exponent"); // LCOV_EXCL_LINE
return kPowerOfTen[exponent];
}
static constexpr CuspRoundingFix
@@ -575,8 +622,7 @@ public:
template <
auto MinMantissa,
auto MaxMantissa,
Integral64 T = std::decay_t<decltype(MinMantissa)>,
Integral64 TMax = std::decay_t<decltype(MaxMantissa)>>
Integral64 T = std::decay_t<decltype(MinMantissa)>>
[[nodiscard]]
std::pair<T, int>
normalizeToRange() const;
@@ -637,7 +683,8 @@ private:
int& exponent,
MantissaRange::rep const& minMantissa,
MantissaRange::rep const& maxMantissa,
MantissaRange::CuspRoundingFix cuspRoundingFixEnabled);
MantissaRange::CuspRoundingFix cuspRoundingFixEnabled,
bool dropped);
[[nodiscard]]
bool
@@ -876,16 +923,18 @@ Number::isnormal() const noexcept
return isnormal(kRange);
}
template <auto MinMantissa, auto MaxMantissa, Integral64 T, Integral64 TMax>
template <auto MinMantissa, auto MaxMantissa, Integral64 T>
std::pair<T, int>
Number::normalizeToRange() const
{
static_assert(std::is_same_v<T, std::uint64_t> || std::is_same_v<T, std::int64_t>);
static_assert(std::is_same_v<T, TMax>);
static_assert(std::is_same_v<T, std::decay_t<decltype(MinMantissa)>>);
static_assert(std::is_same_v<T, std::decay_t<decltype(MaxMantissa)>>);
auto constexpr kMIN = static_cast<T>(MinMantissa);
auto constexpr kMAX = static_cast<T>(MaxMantissa);
static_assert(kMIN > 0);
static_assert(kMIN % 10 == 0);
static_assert(isPowerOfTen(static_cast<std::make_unsigned_t<T>>(kMIN)));
static_assert(kMAX % 10 == 9);
static_assert((kMAX + 1) / 10 == kMIN);

View File

@@ -15,8 +15,8 @@ Generation requires a one-time setup step to create a virtual environment
and install Python dependencies, followed by running the generation target:
```bash
cmake --build . --target setup_code_gen # create venv and install dependencies (once)
cmake --build . --target code_gen # generate code
cmake --build . --target setup_code_gen # create venv and install dependencies (once)
cmake --build . --target code_gen # generate code
```
By default, `CODEGEN_VENV_DIR` points to `.venv` in the project root. The

View File

@@ -74,10 +74,10 @@ VERSION=2.4.0-local
PKG_RELEASE=1
docker run --rm \
-v "$(pwd):/src" \
-w /src \
"$IMAGE" \
./package/build_pkg.sh --pkg-version "$VERSION" --pkg-release "$PKG_RELEASE"
-v "$(pwd):/src" \
-w /src \
"$IMAGE" \
./package/build_pkg.sh --pkg-version "$VERSION" --pkg-release "$PKG_RELEASE"
# Output:
# build/debbuild/*.deb (DEB + dbgsym .ddeb)
@@ -92,12 +92,12 @@ needed, but the host toolchain replaces the pinned CI image:
```bash
cmake \
-Dxrpld=ON \
-Dxrpld_version=2.4.0-local \
-Dtests=OFF \
..
-Dxrpld=ON \
-Dxrpld_version=2.4.0-local \
-Dtests=OFF \
..
cmake --build . --target package # deb on Debian/Ubuntu, rpm on RHEL
cmake --build . --target package # deb on Debian/Ubuntu, rpm on RHEL
```
The `cmake/XrplPackaging.cmake` module defines the target only if at least one

View File

@@ -179,6 +179,10 @@ public:
setPositive() noexcept;
void
setNegative() noexcept;
// Should only be called by doNormalize, and then only for division
// operations with remainders.
void
setDropped() noexcept;
[[nodiscard]] bool
isNegative() const noexcept;
@@ -251,6 +255,12 @@ Number::Guard::setNegative() noexcept
sbit_ = 1;
}
inline void
Number::Guard::setDropped() noexcept
{
xbit_ = 1;
}
inline bool
Number::Guard::isNegative() const noexcept
{
@@ -397,7 +407,7 @@ Number::Guard::doRoundUp(
// _don't_ increment the mantissa. Instead, divide and round recursively. It should
// be impossible to recurse more than once, because once the mantissa is divided by
// 10, it will be _well_ under maxMantissa and kLargestMantissa, so adding 1 will
// have no change of bringing it back over.
// have no chance of bringing it back over.
doDropDigit(mantissa, exponent);
XRPL_ASSERT_PARTS(
safeToIncrement(mantissa),
@@ -630,7 +640,6 @@ Number::one()
return one(kRange);
}
// Use the member names in this static function for now so the diff is cleaner
template <class T>
void
doNormalize(
@@ -639,7 +648,8 @@ doNormalize(
int& exponent,
MantissaRange::rep const& minMantissa,
MantissaRange::rep const& maxMantissa,
MantissaRange::CuspRoundingFix cuspRoundingFixEnabled)
MantissaRange::CuspRoundingFix cuspRoundingFixEnabled,
bool dropped)
{
auto constexpr kMinExponent = Number::kMinExponent;
auto constexpr kMaxExponent = Number::kMaxExponent;
@@ -663,6 +673,8 @@ doNormalize(
Guard g;
if (negative)
g.setNegative();
if (dropped)
g.setDropped();
while (m > maxMantissa)
{
if (exponent >= kMaxExponent)
@@ -707,7 +719,12 @@ Number::normalize<uint128_t>(
internalrep const& maxMantissa,
MantissaRange::CuspRoundingFix cuspRoundingFixEnabled)
{
doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled);
// Not used by every compiler version, and thus not necessarily
// counted by coverage build
// LCOV_EXCL_START
doNormalize(
negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false);
// LCOV_EXCL_STOP
}
template <>
@@ -720,7 +737,12 @@ Number::normalize<unsigned long long>(
internalrep const& maxMantissa,
MantissaRange::CuspRoundingFix cuspRoundingFixEnabled)
{
doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled);
// Not used by every compiler version, and thus not necessarily
// counted by coverage build
// LCOV_EXCL_START
doNormalize(
negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false);
// LCOV_EXCL_STOP
}
template <>
@@ -733,7 +755,8 @@ Number::normalize<unsigned long>(
internalrep const& maxMantissa,
MantissaRange::CuspRoundingFix cuspRoundingFixEnabled)
{
doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled);
doNormalize(
negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false);
}
void
@@ -942,78 +965,164 @@ Number::operator/=(Number const& y)
return *this;
// n* = numerator
// d* = denominator
// *p = negative (positive?)
// z* = result (quotient)
// *p = negative (p for positive, even though the value means not
// positive?)
// *s = sign
// *m = mantissa
// *e = exponent
auto [np, nm, ne] = toInternal(range);
// Create the mantissas as 128-bit unsigned, since that's what we
// need to work with.
auto const [np, nm, ne] = toInternal<uint128_t>(range);
int const ns = (np ? -1 : 1);
auto [dp, dm, de] = y.toInternal(range);
auto const [dp, dm, de] = y.toInternal<uint128_t>(range);
int const ds = (dp ? -1 : 1);
auto const& minMantissa = range.min;
auto const& maxMantissa = range.max;
auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled;
// Shift by 10^17 gives greatest precision while not overflowing
// uint128_t or the cast back to int64_t
// TODO: Can/should this be made bigger for kLARGE_RANGE?
// log(2^128,10) ~ 38.5
// kLARGE_RANGE.log = 18, fits in 10^19
// f can be up to 10^(38-19) = 10^19 safely
bool const small = range.scale == MantissaRange::MantissaScale::Small;
uint128_t const f = small ? 100'000'000'000'000'000 : 10'000'000'000'000'000'000ULL;
XRPL_ASSERT_PARTS(f >= minMantissa * 10, "Number::operator/=", "factor expected size");
// Division operates on two large integers (16-digit for small
// mantissas, 19-digit for large) using integer math. If the values
// were just divided directly, the result would be only ever be one
// digit or zero - not very useful.
// e.g. 9'876'543'210'987'654 / 1'234'567'890'123'456 = 8
// 1'234'567'890'123'456 / 9'876'543'210'987'654 = 0
// Introduce a power-of-ten multiplication factor for the numerator
// which will ensure the result has a meaningful number of digits.
//
// Consider numbers with a 2-digit mantissa:
// * Assume both numbers have an exponent of 0, using "ToNearest" rounding
// * 23 / 67 = 0
// * Use a factor of 10^4
// * 230'000 / 67 = 3432 with an exponent of -4
// * The normalized result will be 34, exponent -2, or 0.34
//
// The most extreme results are 10/99 and 99/10
// * 100'000 / 99 = 1'010e-4 = 10e-2 or 0.10
// * 990'000 / 10 = 99'000e-4 = 99e-1 or 9.9
//
// Note that the computations give 2 or 3 digits after the
// decimal point to determine which way to round for most scenarios.
//
// For small mantissas (where the MantissaRange.log == 15), shifting by 10^17 gives sufficient
// precision while not overflowing uint128_t or the cast back to int64_t. (This is legacy
// behavior, which must not be changed.)
//
// For large mantissas (where the MantissaRange.log == 18), a shift by 10^20 would be optimal
// for most scenarios. However, larger mantissa values would overflow 2^128.
//
// * log(2^128,10) ~ 38.5
// * largeRange.log = 18, fits in 10^19
// * The expanded numerator must fit in 10^38
// * f not be more than 10^(38-19) = 10^19 safely
//
// So, we do the division into stages:
//
// Stage 1: Use the same factor of 10^17, for the initial division. This
// will frequently not result in a whole number quotient.
//
// Stage 2: If there is a remainder from the first step, repeat the
// process with a "correction" factor of 10^5. Shift the
// result of Stage 1 over by 5 places, and add the second result to it.
// This is equivalent to if we had used an initial factor of 10^22,
// a couple digits more than we actually need.
//
// Stage 3: If there is still a remainder, and the CuspRoundingFix
// is enabled, pass a flag indicating such to doNormalize. The Guard
// in doNormalize will treat that flag as if non-zero digits had
// been dropped from the mantissa when shrinking it into range.
// This is only relevant when rounding away from zero (Upward for
// positive numbers, Downward for negative), or if the "regular"
// remainder is exactly 0.5 for "ToNearest". This will give the
// rounding the most accurate result possible, as if infinite
// precision was used in the initial calculation.
// unsigned denominator
auto const dmu = static_cast<uint128_t>(dm);
// correctionFactor can be anything between 10 and f, depending on how much
// extra precision we want to only use for rounding with the
// kLARGE_RANGE. Three digits seems like plenty, and is more than
// the kSMALL_RANGE uses.
uint128_t const correctionFactor = 1'000;
// Stage 1: Do the initial division with a factor of 10^17.
auto constexpr factorExponent = 17;
uint128_t constexpr f = kPowerOfTen[factorExponent];
auto const numerator = uint128_t(nm) * f;
auto zm = numerator / dmu;
auto ze = ne - de - (small ? 17 : 19);
bool zn = (ns * ds) < 0;
if (!small)
auto zm = numerator / dm;
auto ze = ne - de - factorExponent;
bool zp = (ns * ds) < 0;
// dropped is used in the same way as Guard::xbit_. In the case of
// division, it indicates if there's any remainder left over after
// we have been as precise as reasonable. If there is, it would be as
// if we were using infinite precision math, and a non-zero digit
// had been shifted off the end of the result when normalizing.
bool dropped = false;
if (range.scale != MantissaRange::MantissaScale::Small)
{
// Virtually multiply numerator by correctionFactor. Since that would
// overflow in the existing uint128_t, we'll do that part separately.
// Stage 2
//
// If there is a remainder, treat it as a secondary numerator.
// Multiply by correctionFactor separately from stage 1.
// The math for this would work for small mantissas, but we need to
// preserve existing behavior.
// preserve legacy behavior.
//
// Consider:
// ((numerator * correctionFactor) / dmu) / correctionFactor
// = ((numerator / dmu) * correctionFactor) / correctionFactor)
// ((numerator * correctionFactor) / dm) / correctionFactor
// = ((numerator / dm) * correctionFactor) / correctionFactor)
//
// But that assumes infinite precision. With integer math, this is
// equivalent to
//
// = ((numerator / dmu * correctionFactor)
// + ((numerator % dmu) * correctionFactor) / dmu) / correctionFactor
// = ((numerator / dm * correctionFactor)
// + ((numerator % dm) * correctionFactor) / dm) / correctionFactor
// = ((zm * correctionFactor)
// + (remainder * correctionFactor) / dm) / correctionFactor
//
// We have already set `mantissa_ = numerator / dmu`. Now we
// compute `remainder = numerator % dmu`, and if it is
// nonzero, we do the rest of the arithmetic. If it's zero, we can skip
// it.
auto const remainder = (numerator % dmu);
// The trick is that multiplication by correctionFactor is done on the mantissa, but
// division by correctionFactor is done by modifying the exponent, so no precision is lost
// until we normalize.
//
// If remainder is zero, we can skip this stage entirely because
// the first stage gave an exact answer.
auto constexpr correctionExponent = 5;
uint128_t constexpr correctionFactor = kPowerOfTen[correctionExponent];
static_assert(factorExponent + correctionExponent == 22);
auto const remainder = (numerator % dm);
if (remainder != 0)
{
zm *= correctionFactor;
auto const correction = remainder * correctionFactor / dmu;
zm += correction;
// divide by 1000 by moving the exponent, so we don't lose the
// integer value we just computed
ze -= 3;
auto const partialNumerator = remainder * correctionFactor;
auto const correction = partialNumerator / dm;
// If the correction is zero, we do not have to make any
// modifications to z*, because it will not have any
// effect on the final result. (We'd be adding a bunch of
// zeros to the end of zm that would just be removed in
// normalize.) However, if that is the case, then Stage 3 is
// even more important for accuracy.
if (correction != 0)
{
zm *= correctionFactor;
// divide by the correctionFactor by moving the exponent, so we don't lose the
// integer value we just computed
ze -= correctionExponent;
zm += correction;
}
// Stage 3: If there's still anything left, and the cusp
// rounding fix is enabled, flag if there is still
// a remainder from stage 2.
bool const useTrailingRemainder =
cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled;
if (useTrailingRemainder)
{
dropped = partialNumerator % dm != 0;
}
}
}
normalize(zn, zm, ze, minMantissa, maxMantissa, cuspRoundingFixEnabled);
fromInternal(zn, zm, ze, &range);
doNormalize(zp, zm, ze, minMantissa, maxMantissa, cuspRoundingFixEnabled, dropped);
fromInternal(zp, zm, ze, &range);
XRPL_ASSERT_PARTS(isnormal(range), "xrpl::Number::operator/=", "result is normalized");
return *this;

View File

@@ -6,12 +6,15 @@
#include <xrpl/protocol/SystemParameters.h>
#include <xrpl/protocol/XRPAmount.h>
// NOLINTNEXTLINE(misc-include-cleaner)
#include <boost/multiprecision/cpp_dec_float.hpp>
#include <boost/multiprecision/number.hpp>
#include <array>
#include <cctype>
#include <chrono>
#include <cstdint>
#include <iomanip>
#include <limits>
#include <map>
#include <sstream>
@@ -41,6 +44,40 @@ class Number_test : public beast::unit_test::Suite
return out;
}
using dec = boost::multiprecision::cpp_dec_float_50;
template <class T = dec>
static T
pow10(int n)
{
if (n == 0)
return 1;
if (n == 1)
return 10;
if (n > 1)
{
auto r = pow10<T>(n / 2);
r *= r;
if (n % 2 != 0)
r *= 10;
return r;
}
// n < 0
T p = 1;
p /= pow10<T>(-n);
return p;
}
static std::string
fmt(dec const& v)
{
std::ostringstream os;
os << std::setprecision(40) << v;
return os.str();
}
public:
void
testZero()
@@ -1949,39 +1986,249 @@ public:
void
testUpwardRoundsDown()
{
testcase << "upward rounding produces a value below exact at kLargestMantissa cusp";
auto const scale = Number::getMantissaScale();
{
testcase << "upward rounding produces a value below exact at kLargestMantissa cusp"
<< to_string(scale);
NumberMantissaScaleGuard const mg{MantissaRange::MantissaScale::Large};
NumberRoundModeGuard const rg{Number::RoundingMode::Upward};
NumberRoundModeGuard const rg{Number::RoundingMode::Upward};
constexpr std::int64_t kAValue = 1'000'000'000'000'049'863LL;
constexpr std::int64_t kBValue = 9'223'372'036'854'315'903LL;
constexpr std::int64_t kAValue = 1'000'000'000'000'049'863LL;
constexpr std::int64_t kBValue = 9'223'372'036'854'315'903LL;
Number const a = kAValue;
Number const b = kBValue;
Number const product = a * b;
Number const a = kAValue;
Number const b = kBValue;
Number const product = a * b;
// Exact reference in BigInt.
BigInt const exactProduct = BigInt(kAValue) * BigInt(kBValue);
// Exact reference in BigInt.
BigInt const exactProduct = BigInt(kAValue) * BigInt(kBValue);
// What Number actually stored.
BigInt storedValue = BigInt(product.mantissa());
for (int i = 0; i < product.exponent(); ++i)
storedValue *= 10;
// What Number actually stored.
BigInt storedValue = BigInt(product.mantissa());
for (int i = 0; i < product.exponent(); ++i)
storedValue *= 10;
BigInt const signedDifference = storedValue - exactProduct;
BigInt const signedDifference = storedValue - exactProduct;
log << "\n"
<< " a = " << fmt(BigInt(kAValue)) << "\n"
<< " b = " << fmt(BigInt(kBValue)) << "\n"
<< " exact a*b = " << fmt(exactProduct) << "\n"
<< " stored = " << fmt(storedValue) << "\n"
<< " stored - exact = " << fmt(signedDifference) << "\n"
<< " upward = " << (signedDifference >= 0 ? "held" : "VIOLATED") << "\n";
log << "\n"
<< " a = " << fmt(BigInt(kAValue)) << "\n"
<< " b = " << fmt(BigInt(kBValue)) << "\n"
<< " exact a*b = " << fmt(exactProduct) << "\n"
<< " stored = " << fmt(storedValue) << "\n"
<< " stored - exact = " << fmt(signedDifference) << "\n"
<< " upward = " << (signedDifference >= 0 ? "held" : "VIOLATED") << "\n"
<< " stored.mantissa = " << product.mantissa() << "\n"
<< " stored.exponent = " << product.exponent() << "\n";
log.flush();
BEAST_EXPECT(signedDifference >= 0);
BEAST_EXPECT(product.mantissa() == (std::numeric_limits<std::int64_t>::max() / 10) + 1);
BEAST_EXPECT(product.exponent() == 19);
switch (scale)
{
case MantissaRange::MantissaScale::Large:
BEAST_EXPECT(signedDifference >= 0);
BEAST_EXPECT(signedDifference < pow10<BigInt>(product.exponent()));
BEAST_EXPECT(
product.mantissa() == (std::numeric_limits<std::int64_t>::max() / 10) + 1);
BEAST_EXPECT(product.exponent() == 19);
break;
case MantissaRange::MantissaScale::LargeLegacy:
BEAST_EXPECT(signedDifference < 0);
BEAST_EXPECT(
product.mantissa() ==
(std::numeric_limits<std::int64_t>::max() / 100) * 100);
BEAST_EXPECT(product.exponent() == 18);
break;
case MantissaRange::MantissaScale::Small:
// The seemingly weird rounding here is because
// a & b are both normalized, and both round up when
// being converted to Number, so you're really
// getting 1_000_000_000_000_050 * 9_223_372_036_854_316
BEAST_EXPECT(signedDifference >= 0);
BEAST_EXPECT(
product.mantissa() ==
(std::numeric_limits<std::int64_t>::max() / 1000) + 3);
BEAST_EXPECT(product.exponent() == 21);
break;
}
}
{
/* Companion regression for the kMaxRep cusp behavior, but for
* `operator/=` on the cusp-fix-ENABLED `Large` scale.
*
* Before the dropped-remainder fix, `operator/=` with Upward
* rounding could return a value STRICTLY LESS than the exact quotient,
* violating Upward's directional invariant.
*
* Mechanism (fix-enabled path):
* 1. `operator/=` computes `numerator = nm * 10^17` and
* `zm = numerator / dm` (integer division, truncates remainder).
* 2. If `remainder != 0`, the correction block runs:
* zm *= 100000
* correction = (remainder * 100000) / dm // also truncates
* zm += correction
* ze -= 5
* The truncation in `correction` discards a sub-1/100000 residual.
* 3. `normalize`'s shift loop reduces zm to fit, but the discarded
* residual is BELOW the Guard's visibility, so the Guard sees fraction = 0.
* 4. Under Upward + positive, `round()` returns -1 (no round-up), and
* the algorithm returns the truncated zm
*/
testcase << "operator/= Upward on Large returns value < truth " << to_string(scale);
NumberRoundModeGuard const roundGuard{Number::RoundingMode::Upward};
constexpr std::int64_t aValue = 2LL;
constexpr std::int64_t bValue = 1'000'000'000'000'000'007LL;
// bValue = 10^18 + 7 (prime, in [minMantissa, kMaxRep]).
Number const a{aValue, 0};
Number const b{bValue, 0};
Number const quotient = a / b;
dec const exact = dec(aValue) / dec(bValue);
dec const stored = dec(quotient.mantissa()) * pow10(quotient.exponent());
dec const diff = stored - exact;
log << "\n"
<< " a = " << aValue << "\n"
<< " b = " << bValue << "\n"
<< " exact a/b = " << fmt(exact) << "\n"
<< " stored a/b = " << fmt(stored) << "\n"
<< " stored - exact = " << fmt(diff)
<< " (negative => Upward gave value BELOW truth)\n"
<< " quotient.mantissa = " << quotient.mantissa() << "\n"
<< " quotient.exponent = " << quotient.exponent() << "\n";
log.flush();
// Upward invariant: stored >= exact. Bug: stored < exact.
switch (scale)
{
case MantissaRange::MantissaScale::Large:
BEAST_EXPECT(stored >= exact);
BEAST_EXPECT(diff < pow10(quotient.exponent()));
break;
case MantissaRange::MantissaScale::LargeLegacy:
BEAST_EXPECT(stored < exact);
BEAST_EXPECT(diff >= -pow10(quotient.exponent()));
break;
case MantissaRange::MantissaScale::Small:
// Small mantissa doesn't have the correction for
// dropped remainders
BEAST_EXPECT(stored < exact);
break;
}
}
{
/* Companion test case for Upward positive operator/=: Downward negative
*/
testcase << "operator/= Downward on Large returns value < truth " << to_string(scale);
NumberRoundModeGuard const roundGuard{Number::RoundingMode::Downward};
constexpr std::int64_t aValue = -2LL;
constexpr std::int64_t bValue = 1'000'000'000'000'000'007LL;
// bValue = 10^18 + 7 (prime, in [minMantissa, kMaxRep]).
Number const a{aValue, 0};
Number const b{bValue, 0};
Number const quotient = a / b;
dec const exact = dec(aValue) / dec(bValue);
dec const stored = dec(quotient.mantissa()) * pow10(quotient.exponent());
dec const diff = stored - exact;
log << "\n"
<< " a = " << aValue << "\n"
<< " b = " << bValue << "\n"
<< " exact a/b = " << fmt(exact) << "\n"
<< " stored a/b = " << fmt(stored) << "\n"
<< " stored - exact = " << fmt(diff)
<< " (positive => Downward gave value ABOVE truth)\n"
<< " quotient.mantissa = " << quotient.mantissa() << "\n"
<< " quotient.exponent = " << quotient.exponent() << "\n";
log.flush();
// invariant: stored <= exact. Bug: stored > exact.
switch (scale)
{
case MantissaRange::MantissaScale::Large:
BEAST_EXPECT(stored <= exact);
BEAST_EXPECT(diff > -pow10(quotient.exponent()));
break;
case MantissaRange::MantissaScale::LargeLegacy:
BEAST_EXPECT(stored > exact);
BEAST_EXPECT(diff <= pow10(quotient.exponent()));
break;
case MantissaRange::MantissaScale::Small:
// Small mantissa doesn't have the correction for
// dropped remainders
BEAST_EXPECT(stored < exact);
break;
}
}
{
/* Companion test case for Upward positive operator/=: ToNearest
*
* With ToNearest, if the dropped digits are exactly "5", then the mantissa will be
* rounded to even. The numbers below result in a value where the unrounded mantissa
* ends in an even digit, and "infinite precision" would drop
* "500000000000000000145...", but doNormalize only sees "5". Without the rounding fix,
* doNormalize rounds down to the even value. With the rounding fix, doNormalize knows
* there are more digits beyond "5", and so rounds _up_ to the odd value.
*/
testcase << "operator/= ToNearest on Large returns value < truth " << to_string(scale);
NumberRoundModeGuard const roundGuard{Number::RoundingMode::ToNearest};
constexpr std::int64_t aValue = 1'269'917'268'816'087'809LL;
constexpr std::int64_t bValue = 3'458'525'013'821'685'511LL;
// bValue = 10^18 + 7 (prime, in [minMantissa, kMaxRep]).
Number const a{aValue, 0};
Number const b{bValue, 0};
Number const quotient = a / b;
dec const exact = dec(aValue) / dec(bValue);
dec const stored = dec(quotient.mantissa()) * pow10(quotient.exponent());
dec const diff = stored - exact;
log << "\n"
<< " a = " << aValue << "\n"
<< " b = " << bValue << "\n"
<< " exact a/b = " << fmt(exact) << "\n"
<< " stored a/b = " << fmt(stored) << "\n"
<< " stored - exact = " << fmt(diff)
<< " (negative => ToNearest gave value BELOW truth)\n"
<< " quotient.mantissa = " << quotient.mantissa() << "\n"
<< " quotient.exponent = " << quotient.exponent() << "\n";
log.flush();
// invariant: stored >= exact. Bug: stored < exact.
switch (scale)
{
case MantissaRange::MantissaScale::Large:
BEAST_EXPECT(stored >= exact);
BEAST_EXPECT(diff < pow10(quotient.exponent()));
break;
case MantissaRange::MantissaScale::LargeLegacy:
BEAST_EXPECT(stored < exact);
BEAST_EXPECT(diff >= -pow10(quotient.exponent()));
break;
case MantissaRange::MantissaScale::Small:
// Small mantissa doesn't have the correction for
// dropped remainders
BEAST_EXPECT(stored < exact);
break;
}
}
}
void
@@ -2012,9 +2259,9 @@ public:
testRounding();
testInt64();
testNormalizeToRange();
testUpwardRoundsDown();
}
// This test sets its own number range
testUpwardRoundsDown();
}
};