Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposed 2.0.0-b3 #4772

Merged
merged 19 commits into from
Oct 17, 2023
Merged

Proposed 2.0.0-b3 #4772

merged 19 commits into from
Oct 17, 2023

Conversation

intelliot
Copy link
Collaborator

High Level Overview of Change

This is a beta for the next release. This PR's diff shows the 18 changes since the last beta release (2.0.0-b2).

Highlights:

The base branch is release because all releases (including betas) go in release.

Context of Change

When finalized, the next stable release of rippled is expected to be the last one for 2023. The major version is bumped due to:

  • General availability of api_version 2 (expected).
    • This api_version is not available yet (except in beta, requiring a configuration change), but it will be in the next release.
  • Notable performance improvements in TPS (transactions per second).
  • XLS-38 sidechains support
    • This is a large change and adds significant new functionality to the protocol.
  • Lastly, lots of code has been refactored and changed since the previous major version bump.

Type of Change

  • Release

nbougalis and others added 19 commits September 26, 2023 17:35
Fix the Windows build by using `unsigned int` (instead of `uint`).

The error, introduced by XRPLF#4618, looks something like:
  rpc\impl\RPCHelpers.h(299,5): error C2061: syntax error: identifier
  'uint' (compiling source file app\ledger\Ledger.cpp)
…LF#4410)

Amendment "flapping" (an amendment repeatedly gaining and losing
majority) usually occurs when an amendment is on the verge of gaining
majority, and a validator not in favor of the amendment goes offline or
loses sync. This fix makes two changes:

1. The number of validators in the UNL determines the threshold required
   for an amendment to gain majority.
2. The AmendmentTable keeps a record of the most recent Amendment vote
   received from each trusted validator (and, with `trustChanged`, stays
   up-to-date when the set of trusted validators changes). If no
   validation arrives from a given validator, then the AmendmentTable
   assumes that the previously-received vote has not changed.

In other words, when missing an `STValidation` from a remote validator,
each server now uses the last vote seen. There is a 24 hour timeout for
recorded validator votes.

These changes do not require an amendment because they do not impact
transaction processing, but only the threshold at which each individual
validator decides to propose an EnableAmendment pseudo-transaction.

Fix XRPLF#4350
Modify the `XChainBridge` amendment.

Before this patch, two door accounts on the same chain could could own
the same bridge spec (of course, one would have to be the issuer and one
would have to be the locker). While this is silly, it does not violate
any bridge invariants. However, on further review, if we allow this then
the `claim` transactions would need to change. Since it's hard to see a
use case for two doors to own the same bridge, this patch disallows
it. (The transaction will return tecDUPLICATE).
Update minimum compiler requirement for building the codebase. The
feature "using enum" is required. This feature was introduced in C++20.

Updating the C++ compiler to version 11 or later fixes this error:

```
Building CXX object CMakeFiles/xrpl_core.dir/src/ripple/protocol/impl/STAmount.cpp.o
/build/ripple/binary/src/ripple/protocol/impl/STAmount.cpp: In lambda function:
/build/ripple/binary/src/ripple/protocol/impl/STAmount.cpp:1577:15: error: expected nested-name-specifier before 'enum'
 1577 |         using enum Number::rounding_mode;
      |               ^~~~
```

Fix XRPLF#4693
Address a stack-use-after-scope issue when using rvalues with
`soci::use`. Replace rvalues with lvalues to ensure the scope extends
beyond the end of the expression.

The issue arises from `soci` taking a reference to the rvalue without
copying its value or extending its lifetime. `soci` references rvalues
in `soci::use_container` and then the address in `soci_use_type`. For
types like `int`, memory access post-lifetime is unlikely to cause
issues. However, for `std::string`, the backing heap memory can be freed
and potentially reused, leading to a potential segmentation fault.

This was detected on x86_64 using clang-15 with asan. asan confirms
resolution of the issue.

Fix XRPLF#4675
Make transactions and pseudo-transactions share the same commonFields
again. This regularizes the code in a nice way.

While this technically allows pseudo-transactions to have a
TicketSequence field, pseudo-transactions are only ever constructed by
code paths that don't add such a field, so this is not a transaction
processing change. It may be possible to add a separate check to ensure
TicketSequence (and other fields that don't make sense on
pseudo-transactions) are never added to pseudo-transactions, but that
should not be necessary. (TicketSequence is not the only common field
that can not and does not appear in pseudo-transactions.) Note:
TicketSequence is already documented as a common field.

Related: XRPLF#4637

Fix XRPLF#4714
When a new transactor is added, there are several places in applySteps
that need to be modified. This patch refactors the code so only one
function needs to be modified.
…F#4721)

Context: The `DisallowIncoming` amendment provides an option to block
incoming trust lines from reaching your account. The
asfDisallowIncomingTrustline AccountSet Flag, when enabled, prevents any
incoming trust line from being created. However, it was too restrictive:
it would block an issuer from authorizing a trust line, even if the
trust line already exists. Consider:

1. Issuer sets asfRequireAuth on their account.
2. User sets asfDisallowIncomingTrustline on their account.
3. User submits tx to SetTrust to Issuer.

At this point, without `fixDisallowIncomingV1` active, the issuer would
not be able to authorize the trust line.

The `fixDisallowIncomingV1` amendment, once activated, allows an issuer
to authorize a trust line even after the user sets the
asfDisallowIncomingTrustline flag, as long as the trust line already
exists.
P2P link compression is a feature added in 1.6.0 by XRPLF#3287.

https://xrpl.org/enable-link-compression.html

If the default changes in the future - for example, as currently
proposed by XRPLF#4387 - the comment will be updated at that time.

Fix XRPLF#4656
* Add a new API Changelog section for release 1.10.
* Mark `jss::fee_ref` as deprecated.
* Fix a copy-paste error in one of the unit tests.
Artifactory support was added to the `nix` builds with XRPLF#4556. This
extends that support to the Windows build. Now the Windows build works;
CI will build and test a Windows release build. This only affects CI and
does not change any C++ code.

* Copy the remote setup step outcome fix from XRPLF#4716 discussion
* Allow the Windows job to succeed if tests fail:
  * Currently the tests do not always pass, even on a single threaded
    run on the GitHub runners. So we are using parallel runs and mark
    the test step as allowed to fail (continue-on-error).
  * At this point, it's more important that the build succeeds than that
    the tests succeed, because:
  * We've got plenty of test coverage on the other jobs.
  * Test failures are much rarer than build failures because of
    cross-platform issues.
  * Having a test failure locally doesn't interrupt a workflow nearly as
    much as a build failure.

Note that Conan Center cannot hold the binaries we need. They do not
build the configurations we need, and they will not add them.

## Future Tasks

This introduces a new bottleneck since the build and test takes over an
hour. Speed up the job by:

* Making this job run on heavy Windows runners.
* Increasing the number of hardware threads.
…#4746)

Update the nix CI runner. This commit does not modify any source code
files. The unix builds were successful, but the binaries were not
uploaded to the internal artifactory. This PR borrows an idea from
@ximinez to attempt to fix this issue.

After successful authentication, the `outcome` variable contains a
string. In the upload step, we are checking if outcome == 'success' as a
prerequisite for uploading the binary.

This commit updates the contents of the `outcome` variable.
The assert is saying that the only reason `pathFinder` would be null is
if the request was aborted (connection dropped, etc.). That's what
`continueCallback()` checks. But that is very clearly not true if you
look at `getPathFinder`, which calls `findPaths`, which can return false
for many reasons.

Fix XRPLF#4744
In Windows, we need to call `python` in order for the `pip` upgrade
command to work.

This changes the GitHub Actions Windows CI job to use the correct
command to upgrade PIP, fixing this error:

```
ERROR: To modify pip, please run the following command:
C:\hostedtoolcache\windows\Python\3.9.13\x64\python.exe -m pip install --upgrade pip
```

A future task is to make job run on heavy Windows runners so that it
doesn't take so long.

Context: XRPLF#4596
@intelliot
Copy link
Collaborator Author

Completed successful manual tests by seelabs, scottschurr, ximinez. Typical process is to make release and/or debug builds for Linux, macOS, Windows - and sync to Mainnet. Stay synced for one minute and then stop gracefully.

@intelliot intelliot merged commit 1fc1eb9 into XRPLF:release Oct 17, 2023
25 checks passed
@intelliot intelliot deleted the develop-next branch October 17, 2023 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.