I'm not entirely sure how this happens, but I came across a node that
was failing here because the trigger didn't exist -- possibly from a
partial upgrade?
In any case, the `IF EXISTS` makes it more lenient.
The CLI wallets `print_locked_stakes` was buggy, overly verbose, and
missing pertinent information. This overhauls it.
Details are below; the quick differences:
- show the staking requirement
- make key image output optional with a +key_images flag on the command
- fix various bugs such as just showing the first contribution as the
"total" contribution
- add the wallet address of extra contributions, rather than just the
key image with no other info.
- put your own contributions first, and mark them as such
- show totals of other contributors
- sort service nodes to put unlocking nodes first; among unlocking and
non-unlocking nodes we sort by hex pubkey string.
- sort locked dereg outputs by unlock height
- considerable reformatting of how things are displayed
---
Output now looks like this:
First we have 1-2 lines of general info:
Service Node: abcdef123456...
Unlock Height: 1234567 (omitted if not unlocking)
If there are other contributors then we print a total line such as:
Total Contributions: 15000 OXEN of 15000 OXEN required
For our own contribution, when we have a single contribution, we use one of:
Your Contribution: 5000 OXEN (Key image: abcdef123...)
Your Contribution: 5000 OXEN of 15000 OXEN required (Key image: abcdef123...)
(the second one applies if we are the only contributor so far).
If we made multiple contributions then:
Your Contributions: 5000 OXEN in 2 contributions:
Your Contributions: 5000 OXEN of 15000 OXEN required in 2 contributions:
(the second one if we are the only contributor so far).
This is followed by the individual contributions:
‣ 4000.5 OXEN (Key image: abcdef123...)
‣ 999.5 OXEN (Key image: 789cba456...)
If there are other contributors then we also print:
Other contributions: 10000 OXEN from 2 contributors:
• 1234.565 OXEN (T6U7YGUcPJffbaF5p8NLC3VidwJyHSdMaGmSxTBV645v33CmLq2ZvMqBdY9AVB2z8uhbHPCZSuZbv68hE6NBXBc51Gg9MGUGr)
Key image 123456789...
• 8765.435 OXEN (T6Tpop5RZdwE39iBvoP5xpJVoMpYPUwQpef9zS2tLL8yVgbppBbtGnzZxzkSp53Coi88wbsTHiokr7k8MQU94mGF1zzERqELK)
‣ 7530 OXEN (Key image: 23456789a...)
‣ 1235.435 OXEN (Key image: 3456789ab...)
If we aren't showing key images then all the key image details get omitted.
---
Locked key images get overhauled too; it wasn't at all clear from the
output *why* these are locked (i.e. these are locked contributions of
failed SNs):
Locked Stakes due to Service Node Deregistration:
‣ 234.567 OXEN (Unlock height 1234567; Key image: abcfed999...)
‣ 5000 OXEN (Unlock height 123333; Key image: cbcfef989...)
With batching, individual blocks can have a negative coinbase emission
because the tx fee gets added to the batch rewards database and not paid
out immediately, which then results in an negative overflow to a value
close to 2^64. Thus a block with no payout and a tx fee will have an
erroneous huge positive coinbase emission when queried via
`get_coinbase_tx_sum`. For example block 1094068 queried with:
{"jsonrpc":"2.0","id":"0","method":"get_coinbase_tx_sum","params":{"height": 1094068, "count": 1}}
returns:
{
"jsonrpc": "2.0",
"id": "0",
"result": {
"burn_amount": 0,
"emission_amount": 18446744073699378616,
"fee_amount": 10173000,
"status": "OK"
}
}
This commit fixes it by making the values signed (and also serves as an
example of why unsigned integers are usually the wrong choice):
{
"jsonrpc": "2.0",
"id": "0",
"result": {
"burn_amount": 0,
"emission_amount": -10173000,
"fee_amount": 10173000,
"status": "OK"
}
}
Profiling shows noticeable CPU spend in allocating memory for this
vector (which makes sense, since we are looping through ~1700 nodes and
building a reward vector for each one). Avoid it by reusing a single
vector that gets cleared (but not reallocated more than a handful of
times).
This reduces batching CPU time in a debug build by about 12%; curiously
I didn't find a noticeable reduction in a release build.
Use intrinsic 128-bit types when supported by the compiler; the
operations on them are a bit faster than the two-uint64_t
implementations we do not, and this shaves about 7% off the batch
processing time (which makes a lot of mul128_div64 calls).
Also removes a bunch of unused, useless epee crap from int-util.h.
Converting between internal addresses and string wallet addresses is
surprisingly expensive (partly because a keccak hash is involved);
normally on the blockchain that doesn't matter because string
conversions are rare, but batching exposed some code paths that do the
conversion a lot.
This makes two optimizations to considerably speed up batching
processing time:
1. Add an address string hash for batch addresses. We don't see that
many, but we tend to see the same addresses for *every* block. By
caching it we can considerably reduce the time needed to do the
actual conversions.
2. In many places we don't really need strings at all: we can work just
with the address_infos (which are cheap). This eliminates some of
the places we use addresses and pushes address_infos further through
the code.
In total, this plus the previous optimization drop batch processing time
by this cuts batch processing CPU time by about 85%, roughly half from
this optimization and roughly half from the store-offsets optimization.
On a debug build, timing mainnet resyncing from just before HF19 to
current:
- neither optimization: 223.60s spent processing batch jobs (of 282s
total recalculating subsystems).
- just offsets: 131s spent in batch processing
- both optimizations: 33s spent in batch processing (down to 60s total
recalculating).
On a release build:
- before: 107.35s (4.73s snl; 95.55s batch)
- after: 28.27s (4.66s snl; 0.00s ons; 21.72s batch)
Batch timing was being reported as 0s, even though it is taking up the
vast majority of the time as of the HF block.
Reduce the block processing size to 500 blocks at a time rather than
1000.
Change when we print status updates to print once duration passes a 10s
threshold rather than every 10*1000 blocks.
This latter change probably also makes oxend more reliable on
low-powered servers because it also guards the systemd status update
message (which is needed to give us more watchdog time).
When the sqlite batching database is resyncing it is dependant on the
service node list state being correct at the time the block is added to
the batching database. When syncing a singular module this sometimes is
not the case as the service node list state may be ahead or behind. This
patch simply ensures that when the batching database is behind the
service node list is reset back to that point in time.
Batching code relies on UPSERTs which were added in 3.24.
(This broke initial bionic debs, which needed a sqlite3 backport from
newer Ubuntu to address).
The RPC call to return accrued balances currently returns some 0s,
because the database has sub-atomic (but non-zero) stored amounts.
This commit skips those when querying accrued earnings.
wallet2 currently exposes a bug where it shows the next payout being +1
interval blocks from now one block too soon (i.e. if I earn a reward on
block 1234 then it should give me 1234 as my next payout height up until
we actually get block 1234 with the payout).
db_sqlite.cpp was already getting this behaviour by passing in height-1,
but that feels like a workaround for an off-by-one in the function
itself. This fixes the function (and removes the -1 from
db_sqlite.cpp).
- When calculating the offset the calculation here is overflowing; since
they are `uint64_t`s where overflow is well-defined that doesn't
actually break anything (the overflowed value overflows in the other
direction when added, and works as the intended subtraction), but it
feels a bit icky. Rearranging the logic avoids it.
- If given a small height (e.g. 3) the next payout height would come out
as a large positive number (close to uint64_t max). The change above
avoids this as well.
Unlike a vector, if a std::array doesn't exist in the response epee
chokes. This fixes it to make it silently ignore it (and leave the
array values untouched), which just makes it consistent with what it
does for any other type of field.
This is particularly needed to be able to retrieve service_node_info
with the `service_node_version`, `lokinet_version`, and
`storage_server_version` fields omitted without getting a
deserialization failure from the omission.
This adds detail to the balance command of the CLI wallet to show how
much has been batched for each address and also summarises when the next
payout is.
It also includes additional fields to get_balance rpc endpoint on the
oxen-wallet-rpc so the user can check on the status of their accrued
funds.
If we pass the blob into parse_and_validate_tx_from_blob we get
`deserialization or varint failed` error logs, which are harmless bug
annoying. This catches the empty tx blob case earlier and logs at a
lesser severity.
If on_transaction_relayed fails to parse, it returns a null_hash, but
that wasn't being detected here.
Also eliminates calling `on_transaction_relayed(tx_blob)` twice.
Some node on the network apparently has a corrupted DB and is spewing
empty tx blobs onto the network. Detect such a case rather than
broadcasting broken empty txes.
Jason Rhinelander
Thomas Winget
Sean Darcy