-
Notifications
You must be signed in to change notification settings - Fork 296
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
multi: Add ticket exhaustion check. #2398
Conversation
Testing this might be a little bit challenging for the uninitiated, so here is a guide:
harness script diffdiff --git a/contrib/dcr_tmux_simnet_setup.sh b/contrib/dcr_tmux_simnet_setup.sh
index bdc463a5..78fc11d8 100755
--- a/contrib/dcr_tmux_simnet_setup.sh
+++ b/contrib/dcr_tmux_simnet_setup.sh
@@ -133,7 +133,7 @@ tmux resize-pane -D 15
tmux send-keys "cd ${NODES_ROOT}/${PRIMARY_WALLET_NAME}" C-m
tmux send-keys "echo \"${WALLET_CREATE_CONFIG}\" | dcrwallet -C ../wallet.conf --create; tmux wait-for -S ${PRIMARY_WALLET_NAME}" C-m
tmux wait-for ${PRIMARY_WALLET_NAME}
-tmux send-keys "dcrwallet -C ../wallet.conf --enableticketbuyer --ticketbuyer.limit=10" C-m
+tmux send-keys "dcrwallet -C ../wallet.conf" C-m
tmux select-pane -t 1
tmux send-keys "cd ${NODES_ROOT}/${PRIMARY_WALLET_NAME}" C-m
$ ./directtickets 5 NOTE: [1]: Unfortunately, it's not possible to use the regular directtickets script#!/usr/bin/env bash
NUM=1
case $1 in
''|*[!0-9]*) ;;
*) NUM=$1 ;;
esac
stakediff=$(./ctl getstakedifficulty | jq '.current * 1e8 | round')
utxos=$(./ctl listunspent | jq "map(select(.spendable==true and (.amount*1e8 | round)>${stakediff}))")
numutxos=$(echo $utxos | jq 'length')
[[ ${numutxos} -ge ${NUM} ]] || { echo "not enough unspent outputs"; exit 1; }
votingaddr=$(./ctl getnewaddress)
commitaddr=$(./ctl getnewaddress)
fee=2970
for i in $(seq 1 1 ${NUM}); do
utxo=$(echo "$utxos" | jq ".[$((i - 1))]")
txid=$(echo $utxo | jq -r '.txid')
vout=$(echo $utxo | jq -r '.vout')
tree=$(echo $utxo | jq -r '.tree')
amt=$(echo $utxo | jq -r '.amount*1e8 | round')
changeamt=$(jq -rn "${amt}-${stakediff}-${fee}")
./ctl createrawsstx "[{\"txid\":\"${txid}\",\"vout\":${vout},\"tree\":${tree},\"amt\":${amt}}]" \
"{\"${votingaddr}\": ${stakediff}}" \
"[{\"addr\":\"${commitaddr}\",\"commitamt\":${stakediff},\"changeaddr\":\"${commitaddr}\",\"changeamt\":${changeamt}}]" | \
./ctl signrawtransaction - | jq -r .hex | \
./ctl sendrawtransaction -
done In order to see why this behavior is accurate, run a similar test with the current code on master without this change to actually trigger the exhaustion as follows:
|
80dcc1d
to
923434c
Compare
19596e8
to
19af5ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, excellent test cases. Tested locally as well per the instructions (thanks for adding that, definitely makes it easier to review). Noticed a couple of small typos and other than that this looks good to go to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified the test data and did the functional tests.
Only found a small thing for clarification.
19af5ba
to
00fc91b
Compare
This adds a new function named checkTicketExhaustion which will return a new rule error if extending a given block with another block that contains a specified number of tickets will result in an unrecoverable chain due to inevitable ticket exhaustion along with tests to ensure proper functionality. The approach taken is such that it should be easy to use in a future consensus change gated behind a vote if that is ultimately desired, which I personally think would be a good idea. An exported variant is also provided that takes the hash of the block to extend for use by external callers such as the mining template code.
This modifies the mining template creation logic to make use of the new ability to check if the template would result in an unrecoverable chain due to inevitable ticket exhaustion along with an associated new mining rule error.
00fc91b
to
ccf024f
Compare
This is an alternative approach to #2090 which resolves several of the issues it had and is originally based of an idea mentioned by @matheusd in #2047.
This modifies the mining template creation logic to prevent creation of templates that would result in an unrecoverable chain due to inevitable ticket exhaustion.
It is split into two individual commits as follows:
The first commit adds a new function named
checkTicketExhaustion
toblockchain
which will return a new rule error if extending a given block with another block that contains a specified number of tickets will result in an unrecoverable chain due to inevitable ticket exhaustion along with tests to ensure proper functionality.The approach taken is such that it should be easy to use in a future consensus change gated behind a vote if that is ultimately desired, which I personally think would be a good idea.
An exported variant is also provided that takes the hash of the block to extend for use by external callers such as the mining template code.
The second commit modifies the mining template creation logic to make use of the new ability along with returning an associated new mining rule error.