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

R4R: Offset validator set power calculations by one block #2255

Merged
merged 52 commits into from
Oct 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
73292e0
Start of changes for TM 0.24
cwgoes Sep 3, 2018
14d5e68
More TM 0.24 updates
cwgoes Sep 3, 2018
532c9ee
Client updates for TM 0.24
cwgoes Sep 3, 2018
f45cfb2
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes Sep 6, 2018
8f6c9e1
Pin to Tendermint 0.24.0-rc0
cwgoes Sep 6, 2018
06adc69
Update testcases for TM 0.24.0-rc0
cwgoes Sep 6, 2018
c667c56
More testcase fixes
cwgoes Sep 6, 2018
1f5fe29
Ignore Go src in linter
cwgoes Sep 6, 2018
dd75cd3
Minor cleanup
cwgoes Sep 6, 2018
e013b9f
Check errors
cwgoes Sep 6, 2018
5316be7
Update PENDING.md
cwgoes Sep 7, 2018
549eba0
Add height offsets
cwgoes Sep 7, 2018
2245b07
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes Sep 7, 2018
897c381
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes Sep 7, 2018
9dd4303
Pin final release
cwgoes Sep 7, 2018
b144dc7
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes Sep 7, 2018
f95d26a
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes Sep 8, 2018
08d4f45
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes Sep 8, 2018
1d55673
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes Sep 8, 2018
d85eb98
make format
cwgoes Sep 8, 2018
e7c2a96
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes Sep 26, 2018
e931195
Remove unnecessary changes
cwgoes Sep 26, 2018
1013703
Update context.Verify
cwgoes Sep 26, 2018
da96e93
'make format'
cwgoes Sep 26, 2018
ecc06f4
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes Sep 26, 2018
f11ee67
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes Sep 27, 2018
fd36abf
Use own directory, fix tx size test, remove parseCmnError
cwgoes Sep 27, 2018
f2422cd
Keep code, codespace
cwgoes Sep 27, 2018
3ed2c22
Merge branch 'develop' into cwgoes/update-tendermint-upstream
cwgoes Oct 1, 2018
08fcd99
Merge branch 'cwgoes/update-tendermint-upstream' into cwgoes/nextvalset
cwgoes Oct 2, 2018
26d43ff
Merge branch 'develop' into cwgoes/nextvalset
cwgoes Oct 3, 2018
4fee791
Fix issue from earlier merge
cwgoes Oct 3, 2018
8f4d3a6
Merge branch 'develop' into cwgoes/nextvalset
cwgoes Oct 4, 2018
48672c4
More comments, update sim for NextValSet
cwgoes Oct 4, 2018
8befc3c
Merge branch 'develop' into cwgoes/nextvalset
cwgoes Oct 5, 2018
a2113b2
Address TODOs in slashing tests
cwgoes Oct 5, 2018
018aaeb
Add testcase for slash at negative height
cwgoes Oct 5, 2018
91b755d
Add testcase for failing to sign "extra block"
cwgoes Oct 5, 2018
1f12f20
Testcase fixes, 'make format', negative pre-genesis block
cwgoes Oct 5, 2018
1909d8f
Update slashing period key appropriately
cwgoes Oct 5, 2018
fdc7b4d
Add default CI multi-seed sim
cwgoes Oct 5, 2018
fa3b459
Actually run multi-seed simulation
cwgoes Oct 5, 2018
034163c
Minor fixes
cwgoes Oct 5, 2018
1046831
Don't trap EXIT
cwgoes Oct 5, 2018
b7cb2e4
Slowly start multiple simulations in parallel
cwgoes Oct 5, 2018
482537e
Merge PR #2430: Aggressive slashing simulation & fixes
cwgoes Oct 5, 2018
b625cf8
Cleanup testcase
cwgoes Oct 5, 2018
317be2b
Clarify ValidatorUpdateDelay
cwgoes Oct 5, 2018
1ed338f
Fixup comments
cwgoes Oct 5, 2018
4814ae4
Merge branch 'develop' into cwgoes/nextvalset
cwgoes Oct 8, 2018
9c53f5a
Merge branch 'develop' into cwgoes/nextvalset
cwgoes Oct 9, 2018
71a80bf
'make format'
cwgoes Oct 9, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,24 @@ jobs:
export PATH="$GOBIN:$PATH"
make test_sim_gaia_fast

test_sim_gaia_multi_seed:
<<: *defaults
parallelism: 1
steps:
- attach_workspace:
at: /tmp/workspace
- checkout
- run:
name: dependencies
command: |
export PATH="$GOBIN:$PATH"
make get_vendor_deps
- run:
name: Test multi-seed Gaia simulation
command: |
export PATH="$GOBIN:$PATH"
make test_sim_gaia_multi_seed

test_cover:
<<: *defaults
parallelism: 4
Expand Down Expand Up @@ -240,6 +258,9 @@ workflows:
- test_sim_gaia_fast:
requires:
- setup_dependencies
- test_sim_gaia_multi_seed:
requires:
- setup_dependencies
- test_cover:
requires:
- setup_dependencies
Expand Down
8 changes: 4 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,9 @@ test_sim_gaia_fast:
@echo "Running quick Gaia simulation. This may take several minutes..."
@go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=400 -SimulationBlockSize=200 -SimulationCommit=true -v -timeout 24h

test_sim_gaia_full:
@echo "Running full multi-seed Gaia simulation. This may take awhile!"
@sh scripts/multisim.sh
test_sim_gaia_multi_seed:
@echo "Running multi-seed Gaia simulation. This may take awhile!"
@bash scripts/multisim.sh 10

SIM_NUM_BLOCKS ?= 210
SIM_BLOCK_SIZE ?= 200
Expand Down Expand Up @@ -241,4 +241,4 @@ localnet-stop:
check_tools check_dev_tools get_tools get_dev_tools get_vendor_deps draw_deps test test_cli test_unit \
test_cover test_lint benchmark devdoc_init devdoc devdoc_save devdoc_update \
build-linux build-docker-gaiadnode localnet-start localnet-stop \
format check-ledger test_sim_gaia_nondeterminism test_sim_modules test_sim_gaia_fast test_sim_gaia_slow update_tools update_dev_tools
format check-ledger test_sim_gaia_nondeterminism test_sim_modules test_sim_gaia_fast test_sim_gaia_multi_seed update_tools update_dev_tools
2 changes: 2 additions & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ BREAKING CHANGES
* [x/stake, x/slashing] [#1305](https://github.com/cosmos/cosmos-sdk/issues/1305) - Rename "revoked" to "jailed"
* [x/stake] [#1676] Revoked and jailed validators put into the unbonding state
* [x/stake] [#1877] Redelegations/unbonding-delegation from unbonding validator have reduced time
* [x/slashing] \#1789 Slashing changes for Tendermint validator set offset (NextValSet)
* [x/stake] [\#2040](https://github.com/cosmos/cosmos-sdk/issues/2040) Validator
operator type has now changed to `sdk.ValAddress`
* [x/stake] [\#2221](https://github.com/cosmos/cosmos-sdk/issues/2221) New
Expand All @@ -41,6 +42,7 @@ BREAKING CHANGES
* [x/gov] [#2195] Governance uses BFT Time
* [x/gov] \#2256 Removed slashing for governance non-voting validators
* [simulation] \#2162 Added back correct supply invariants
* [x/slashing] \#2430 Simulate more slashes, check if validator is jailed before jailing
* [x/stake] \#2393 Removed `CompleteUnbonding` and `CompleteRedelegation` Msg types, and instead added unbonding/redelegation queues to endblocker

* SDK
Expand Down
3 changes: 2 additions & 1 deletion client/keys/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ func AddNewKeyRequestHandler(indent bool) http.HandlerFunc {
PostProcessResponse(w, cdc, keyOutput, indent)
}
}

// function to just a new seed to display in the UI before actually persisting it in the keybase
func getSeed(algo keys.SigningAlgo) string {
kb := client.MockKeyBase()
Expand Down Expand Up @@ -341,4 +342,4 @@ func RecoverRequestHandler(indent bool) http.HandlerFunc {

PostProcessResponse(w, cdc, keyOutput, indent)
}
}
}
2 changes: 1 addition & 1 deletion client/keys/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ func errMissingPassword() error {

func errMissingSeed() error {
return fmt.Errorf("you have to specify seed for key recover")
}
}
2 changes: 1 addition & 1 deletion client/keys/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import (

"github.com/cosmos/cosmos-sdk/client"

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"net/http"
"github.com/cosmos/cosmos-sdk/codec"
)

// KeyDBName is the directory under root where we store the keys
Expand Down
2 changes: 1 addition & 1 deletion client/rpc/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import (

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/context"
"github.com/cosmos/cosmos-sdk/client/utils"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/spf13/viper"
tmtypes "github.com/tendermint/tendermint/types"
"github.com/cosmos/cosmos-sdk/client/utils"
)

// TODO these next two functions feel kinda hacky based on their placement
Expand Down
21 changes: 13 additions & 8 deletions scripts/multisim.sh
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
#!/bin/bash

seeds=(1 2 4 7 9 20 32 123 4728 37827 981928 87821 891823782 989182 89182391)
seeds=(1 2 4 7 9 20 32 123 124 582 1893 2989 3012 4728 37827 981928 87821 891823782 989182 89182391)
blocks=$1

echo "Running multi-seed simulation with seeds: ${seeds[@]}"
echo "Running multi-seed simulation with seeds ${seeds[@]}"
echo "Running $blocks blocks per seed"
echo "Edit scripts/multisim.sh to add new seeds. Keeping parameters in the file makes failures easy to reproduce."
echo "This script will kill all sub-simulations on SIGINT/SIGTERM/EXIT (i.e. Ctrl-C)."
echo "This script will kill all sub-simulations on SIGINT/SIGTERM (i.e. Ctrl-C)."

trap 'kill $(jobs -pr)' SIGINT SIGTERM EXIT
trap 'kill $(jobs -pr)' SIGINT SIGTERM

tmpdir=$(mktemp -d)
echo "Using temporary log directory: $tmpdir"
Expand All @@ -16,7 +18,7 @@ sim() {
echo "Running full Gaia simulation with seed $seed. This may take awhile!"
file="$tmpdir/gaia-simulation-seed-$seed-date-$(date -Iseconds -u).stdout"
echo "Writing stdout to $file..."
go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=1000 \
go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=$blocks \
-SimulationVerbose=true -SimulationCommit=true -SimulationSeed=$seed -v -timeout 24h > $file
}

Expand All @@ -26,7 +28,7 @@ for seed in ${seeds[@]}; do
sim $seed &
pids[${i}]=$!
i=$(($i+1))
sleep 0.1 # start in order, nicer logs
sleep 10 # start in order, nicer logs
done

echo "Simulation processes spawned, waiting for completion..."
Expand All @@ -37,10 +39,13 @@ i=0
for pid in ${pids[*]}; do
wait $pid
last=$?
if [ $last -ne 0 ]; then
seed=${seeds[${i}]}
seed=${seeds[${i}]}
if [ $last -ne 0 ]
then
echo "Simulation with seed $seed failed!"
code=1
else
echo "Simulation with seed $seed OK"
fi
i=$(($i+1))
done
Expand Down
6 changes: 3 additions & 3 deletions x/distribution/types/validator_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ func NewValidatorDistInfo(operatorAddr sdk.ValAddress, currentHeight int64) Vali
return ValidatorDistInfo{
OperatorAddr: operatorAddr,
FeePoolWithdrawalHeight: currentHeight,
Pool: DecCoins{},
PoolCommission: DecCoins{},
DelAccum: NewTotalAccum(currentHeight),
Pool: DecCoins{},
PoolCommission: DecCoins{},
DelAccum: NewTotalAccum(currentHeight),
}
}

Expand Down
2 changes: 1 addition & 1 deletion x/mock/simulation/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const (
numKeys int = 250

// Chance that double-signing evidence is found on a given block
evidenceFraction float64 = 0.01
evidenceFraction float64 = 0.5

// TODO Remove in favor of binary search for invariant violation
onOperation bool = false
Expand Down
8 changes: 6 additions & 2 deletions x/mock/simulation/random_simulate_blocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ func SimulateFromSeed(tb testing.TB, app *baseapp.BaseApp,
}

validators := initChain(r, accs, setups, app, appStateFn)
// Second variable to keep pending validator set (delayed one block since TM 0.24)
// Initially this is the same as the initial validator set
nextValidators := validators

header := abci.Header{Height: 0, Time: timestamp}
opCount := 0
Expand Down Expand Up @@ -160,8 +163,9 @@ func SimulateFromSeed(tb testing.TB, app *baseapp.BaseApp,
// Generate a random RequestBeginBlock with the current validator set for the next block
request = RandomRequestBeginBlock(r, validators, livenessTransitionMatrix, evidenceFraction, pastTimes, pastVoteInfos, event, header)

// Update the validator set
validators = updateValidators(tb, r, validators, res.ValidatorUpdates, event)
// Update the validator set, which will be reflected in the application on the next block
validators = nextValidators
nextValidators = updateValidators(tb, r, validators, res.ValidatorUpdates, event)
}
if stopEarly {
DisplayEvents(events)
Expand Down
28 changes: 22 additions & 6 deletions x/slashing/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/params"
stake "github.com/cosmos/cosmos-sdk/x/stake/types"
abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/crypto"
)
Expand Down Expand Up @@ -56,19 +57,28 @@ func (k Keeper) handleDoubleSign(ctx sdk.Context, addr crypto.Address, infractio
// Double sign confirmed
logger.Info(fmt.Sprintf("Confirmed double sign from %s at height %d, age of %d less than max age of %d", pubkey.Address(), infractionHeight, age, maxEvidenceAge))

// We need to retrieve the stake distribution which signed the block, so we subtract ValidatorUpdateDelay from the evidence height.
// Note that this *can* result in a negative "distributionHeight", up to -ValidatorUpdateDelay,
// i.e. at the end of the pre-genesis block (none) = at the beginning of the genesis block.
// That's fine since this is just used to filter unbonding delegations & redelegations.
distributionHeight := infractionHeight - stake.ValidatorUpdateDelay
Copy link
Member

Choose a reason for hiding this comment

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

I probably need to dig deeper into the slashing mechanics to confirm this, but the infractionHeight is the actual height at which the validator double signed, and is the height at which we want to know the validator set. Are we tracking validator sets differently here? If the double signing was at height H, here we're looking at the validator set at height H-1


// Cap the amount slashed to the penalty for the worst infraction
// within the slashing period when this infraction was committed
fraction := k.SlashFractionDoubleSign(ctx)
revisedFraction := k.capBySlashingPeriod(ctx, consAddr, fraction, infractionHeight)
revisedFraction := k.capBySlashingPeriod(ctx, consAddr, fraction, distributionHeight)
logger.Info(fmt.Sprintf("Fraction slashed capped by slashing period from %v to %v", fraction, revisedFraction))

// Slash validator
k.validatorSet.Slash(ctx, consAddr, infractionHeight, power, revisedFraction)
k.validatorSet.Slash(ctx, consAddr, distributionHeight, power, revisedFraction)

// Jail validator
k.validatorSet.Jail(ctx, consAddr)
// Jail validator if not already jailed
validator := k.validatorSet.ValidatorByConsAddr(ctx, consAddr)
if !validator.GetJailed() {
k.validatorSet.Jail(ctx, consAddr)
}

// Set validator jail duration
// Set or updated validator jail duration
signInfo, found := k.getValidatorSigningInfo(ctx, consAddr)
if !found {
panic(fmt.Sprintf("Expected signing info for validator %s but not found", consAddr))
Expand Down Expand Up @@ -123,7 +133,13 @@ func (k Keeper) handleValidatorSignature(ctx sdk.Context, addr crypto.Address, p
// Downtime confirmed: slash and jail the validator
logger.Info(fmt.Sprintf("Validator %s past min height of %d and below signed blocks threshold of %d",
pubkey.Address(), minHeight, k.MinSignedPerWindow(ctx)))
k.validatorSet.Slash(ctx, consAddr, height, power, k.SlashFractionDowntime(ctx))
// We need to retrieve the stake distribution which signed the block, so we subtract ValidatorUpdateDelay from the evidence height,
// and subtract an additional 1 since this is the LastCommit.
// Note that this *can* result in a negative "distributionHeight" up to -ValidatorUpdateDelay-1,
// i.e. at the end of the pre-genesis block (none) = at the beginning of the genesis block.
// That's fine since this is just used to filter unbonding delegations & redelegations.
rigelrozanski marked this conversation as resolved.
Show resolved Hide resolved
distributionHeight := height - stake.ValidatorUpdateDelay - 1
Copy link
Member

Choose a reason for hiding this comment

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

height - 1 is correct, but same comment as before around also subtracting the update delay.

k.validatorSet.Slash(ctx, consAddr, distributionHeight, power, k.SlashFractionDowntime(ctx))
k.validatorSet.Jail(ctx, consAddr)
signInfo.JailedUntil = ctx.BlockHeader().Time.Add(k.DowntimeUnbondDuration(ctx))
} else {
Expand Down
Loading