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 1.9.1-b1 #4161

Merged
merged 14 commits into from
May 11, 2022
Merged

Proposed 1.9.1-b1 #4161

merged 14 commits into from
May 11, 2022

Conversation

manojsdoshi
Copy link
Contributor

If merged this PR will:

close #4155
close #4152
close #4150
close #4149
close #4139
close #4134
close #4111
close #4097
close #4021
close #3965

nbougalis and others added 14 commits May 6, 2022 11:38
This commit addresses minor bugs introduced with commit
6faaa91:

- The number of threads used by the database engine was
  incorrectly clamped to the lower possible value, such
  that the database was effectively operating in single
  threaded mode.

- The number of requests to extract at once was so high
  that it could result in increased latency. The bundle
  size is now limited to 4 and can be adjusted by a new
  configuration option `rq_bundle` in the `[node_db]`
  stanza. This is an advanced tunable and adjusting it
  should not be needed.
Several hard-coded parameters control the behavior of the ledger
acquisition engine. The values of many of these parameters where
set by intuition and have complex and non-intuitive interactions
with each other and other parts of the code.

An earlier commit attempted to adjust several of these parameters
to improve syncing performance; initial testing was promising but
a number of operators reported experiencing syncing and stability
issues with their servers. As a result, this commit reverts parts
of commit 1823506.

This commit further adjusts some tunables so as to increase the
aggressiveness of the ledger acquisition engine.
One of the two versions of the `rngfill` function accepts a pointer
to a buffer and a size (in bytes). The function aims to fill the
provided `buffer` with `size` random bytes. It does this in chunks
of 8 bytes, for long as possible, and then fills any left-over gap
one byte at a time.

To avoid an annoying and incorrect warning about a potential buffer
overflow in the "trailing write", commit 78bc272
used a `#pragma` to instruct the compiler to not generate the incorrect
diagnostic. Unfortunately, this change _also_ eliminated the trailing
write code, which means that, under some cases, the `rngfill` function
would generate between 1 and 7 fewer random bytes than requested.

This problem would only manifest on builds that do not define `__GNUC__`
which, as of this writing, means MSVC.
* "A path is considered invalid if and only if it enters and exits an
  address node through trust lines where No Ripple has been enabled for
  that address." (https://xrpl.org/rippling.html#specifics)
* When loading trust lines for an account "Alice" which was reached
  via a trust line that has the No Ripple flag set on Alice's side, do
  not use or cache any of Alice's trust lines which have the No Ripple
  flag set on Alice's side. For typical "end-user" accounts, this will
  return no trust lines.
The amendment increases the maximum sign of an account's signer
list from 8 to 32.

Like all new features, the associated amendment is configured with
a default vote of "no" and server operators will have to vote for
it explicitly if they believe it is useful.
o Fixes an off-by-one when determining which NFTokenPage an
  NFToken belongs on.
o Improves handling of packed sets of 32 NFTs with
  identical low 96-bits.
o Fixes marker handling by the account_nfts RPC command.
o Tightens constraints of NFTokenPage invariant checks.

Adds unit tests to exercise the fixed cases as well as tests
for previously untested functionality.
@manojsdoshi manojsdoshi changed the title Propose 1.9.1-b1 Proposed 1.9.1-b1 May 10, 2022
Copy link
Collaborator

@scottschurr scottschurr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

@manojsdoshi manojsdoshi merged commit 670bc22 into XRPLF:develop May 11, 2022
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.

9 participants