Fixes#4374
It was possible for a broker to combine a sell and a buy offer from an account that already owns an NFT. Such brokering extracts money from the NFT owner and provides no benefit in return.
With this amendment, the code detects when a broker is returning an NFToken to its initial owner and prohibits the transaction. This forbids a broker from selling an NFToken to the account that already owns the token. This fixes a bug in the original implementation of XLS-20.
Thanks to @nixer89 for suggesting this fix.
Fixes 3 issues:
In the following scenario, an account cannot perform NFTokenAcceptOffer even though it should be allowed to:
- BROKER has < S
- ALICE offers to sell token for S
- BOB offers to buy token for > S
- BROKER tries to bridge the two offers
This currently results in `tecINSUFFICIENT_FUNDS`, but should not because BROKER is not spending any funds in this transaction, beyond the transaction fee.
When trading an NFT using IOUs, and when the issuer of the IOU has any non-zero value set for TransferFee on their account via AccountSet (not a TransferFee on the NFT), and when the sale amount is equal to the total balance of that IOU that the buyer has, the resulting balance for the issuer of the IOU will become positive. This means that the buyer of the NFT was supposed to have caused a certain amount of IOU to be burned. That amount was unable to be burned because the buyer couldn't cover it. This results in the buyer owing this amount back to the issuer. In a real world scenario, this is appropriate and can be settled off-chain.
Currency issuers could not make offers for NFTs using their own currency, receiving `tecINSUFFICIENT_FUNDS` if they tried to do so.
With this fix, they are now able to buy/sell NFTs using their own currency.
* Introduces amendment `XRPFees`
* Convert fee voting and protocol messages to use XRPAmounts
* Includes Validations, Change transactions, the "Fees" ledger object,
and subscription messages
* Improve handling of 0 drop reference fee with TxQ. For use with networks that do not want to require fees
* Note that fee escalation logic is still in place, which may cause the
open ledger fee to rise if the network is busy. 0 drop transactions
will still queue, and fee escalation can be effectively disabled by
modifying the configuration on all nodes
* Change default network reserves to match Mainnet
* Name the new SFields *Drops (not *XRP)
* Reserve SField IDs for Hooks
* Clarify comments explaining the ttFEE transaction field validation
featureDisallowIncoming is a new amendment that would allow users to opt-out of incoming Checks, Payment Channels, NFTokenOffers, and trust lines. This commit includes tests.
Adds four new AccountSet Flags:
1. asfDisallowIncomingNFTOffer
2. asfDisallowIncomingCheck
3. asfDisallowIncomingPayChan
4. asfDisallowIncomingTrustline
Reduce the reserve requirements from 20/5 to 10/2 in line with the current network votes. The requirements of 10/2 have been on the network long enough that new nodes should not still have the old reserve amount.
Co-authored-by: Richard Holland <richard.holland@starstone.co.nz>
It turns out that the feature enabled by the tfTrustLine flag
on an NFTokenMint transaction could be used as a means to
attack the NFToken issuer. Details are in
https://github.com/XRPLF/rippled/issues/4300
The fixRemoveNFTokenAutoTrustLine amendment removes the
ability to set the tfTrustLine flag on an NFTokenMint
transaction.
Closes 4300.
The XLS-20 implementation contained two bugs that would require the
introduction of amendments. This complicates the adoption of XLS-20
by requiring a staggered amendment activation, first of the two fix
amendments, followed by the `NonFungibleTokensV1` amendment.
After consideration, the consensus among node operators is that the
process should be simplified by the introduction of a new amendment
that, if enabled, would behaves as if the `NonFungibleTokensV1` and
the two fix amendments (`fixNFTokenDirV1` and `fixNFTokenNegOffer`)
were activated at once.
This commit implements this proposal; it does not introduce any new
functionality or additional features, above and beyond that offered
by the existing amendments.
The existing code would, incorrectly, allow negative amounts in offers
for non-fungible tokens. Such offers would be handled very differently
depending on the context: a direct offer would fail with an error code
indicating an internal processing error, whereas brokered offers would
improperly succeed.
This commit introduces the `fixNFTokenNegOffer` amendment that detects
such offers during creation and returns an appropriate error code.
The commit also extends the existing code to allow for buy offers that
contain a `Destination` field, so that a specific broker can be set in
the offer.
A few unit tests have historically generated a lot of noise
to the console from log writes. This noise was not useful
and made it harder to locate actual test failures.
By changing the log level of these tests from
- severities::kError to
- severities::kDisabled
it was possible to remove that noise coming from the logs.
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.
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.
* Txs with the same fee level will sort by TxID XORed with the parent
ledger hash.
* The TxQ is re-sorted after every ledger.
* Attempt to future-proof the TxQ tie breaking test
The pathfinding engine built into the code has several configurable
parameters to adjust the depth of the paths indexed and explored.
These parameters can dramatically impact the performance and memory
consumption of a server; higher values can result in resource usage
increasing exponentially.
These default values were decided early and somewhat arbitrarily at
a time when the network and the size of the network state were much
smaller.
This commit adjusts the default values to reduce the depth of paths
to more reasonable levels; unless explicitly overriden, the changes
mean that pathfinding operations will return fewer, shallower paths
than previous versions of the software.
* Sort by fee level (which is the current behavior) then by transaction
ID (hash).
* Edge case when the account at the end of the queue submits a higher
paying transaction to walk backwards and compare against the cheapest
transaction from a different account.
* Use std::if_any to simplify the JobQueue::isOverloaded loop.
The existing calculation would limit the maximum number of threads
that would be created by default to at most 6; this may have been
reasonable a few years ago, but given both the load on the network
as of today and the increase in the number of CPU cores, the value
should be revisited.
This commit, if merged, changes the default calculation for nodes
that are configured as `large` or `huge` to allow for up to twelve
threads.
* Only require adding the new feature names in one place. (Also need to
increment a counter, but a check on startup will catch that.)
* Allows rippled to have the code to support a given amendment, but
not vote for it by default. This allows the amendment to be enabled in
a future version without necessarily amendment blocking these older
versions.
* The default vote is carried with the amendment name in the list of
supported amendments.
* The amendment table is constructed with the amendment and default
vote.
The existing logic involves every server sending every transaction
that it receives to all its peers (except the one that it received
a transaction from).
This commit instead uses a randomized algorithm, where a node will
randomly select peers to relay a given transaction to, caching the
list of transaction hashes that are not relayed and forwading them
to peers once every second. Peers can then determine whether there
are transactions that they have not seen and can request them from
the node which has them.
It is expected that this feature will further reduce the bandwidth
needed to operate a server.
With this amendment, the CheckCash transaction creates a TrustLine
if needed. The change is modeled after offer crossing. And,
similar to offer crossing, cashing a check allows an account to
exceed its trust line limit.
A recent version of clang notes a number of places in range
for loops where the code base was making unnecessary copies
or using const lvalue references to extend lifetimes. This
fixes the places that clang identified.
This commit expands the detection capabilities of the Byzantine
validation detector. Prior to this commit, only validators that
were on a server's UNL were monitored. Now, all the validations
that a server receives are passed through the detector.
The existing class offered several constructors which were mostly
unnecessary. This commit eliminates all existing constructors and
introduces a single new one, taking a `Slice`.
The internal buffer is switched from `std::vector` to `Buffer` to
save a minimum of 8 bytes (plus the buffer slack that is inherent
in `std::vector`) per SHAMapItem instance.
Before this change any non-zero Sequence field was handled as
a non-ticketed transaction, even if a TicketSequence was
present. We learned that this could lead to user confusion.
So the rules are tightened up.
Now if any transaction contains both a non-zero Sequence
field and a TicketSequence field then that transaction
returns a temSEQ_AND_TICKET error code.
The (deprecated) "sign" and "submit" RPC commands are tuned
up so they auto-insert a Sequence field of zero if they see
a TicketSequence in the transaction.
No amendment is needed because this change is going into
the first release that supports the TicketBatch amendment.
