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

Update TM to latest (+add test) #7442

Merged
merged 25 commits into from
Oct 8, 2020
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
92278b4
Add test for /block_results
amaury1093 Oct 2, 2020
7380c1d
Merge branch 'master' into am-7401-blockresults
amaury1093 Oct 2, 2020
4c07e6e
Fix test
amaury1093 Oct 2, 2020
b383532
Merge branch 'master' of ssh://github.com/cosmos/cosmos-sdk into am-7…
amaury1093 Oct 5, 2020
38aed91
Use http RPC client
amaury1093 Oct 5, 2020
1edf106
Fix temporarily block_results
amaury1093 Oct 5, 2020
b8e5cfa
Use init
amaury1093 Oct 5, 2020
62d920f
Add flag back
amaury1093 Oct 5, 2020
b9ba9cd
Remove init
amaury1093 Oct 5, 2020
9ddcf22
Update TM master
amaury1093 Oct 5, 2020
3a43230
Merge branch 'master' into am-7401-blockresults
amaury1093 Oct 5, 2020
0b8f8c1
Merge branch 'master' into am-7401-blockresults
amaury1093 Oct 6, 2020
a904c1b
Merge branch 'master' into am-7401-blockresults
aaronc Oct 6, 2020
9e1d19f
Merge branch 'master' of ssh://github.com/cosmos/cosmos-sdk into am-7…
amaury1093 Oct 7, 2020
48f10db
Address comments
amaury1093 Oct 7, 2020
40482fc
Fix build
amaury1093 Oct 7, 2020
fe3f9cf
require refactor
amaury1093 Oct 7, 2020
c4e200e
Merge branch 'master' into am-7401-blockresults
mergify[bot] Oct 7, 2020
2e87d57
Merge branch 'master' into am-7401-blockresults
mergify[bot] Oct 7, 2020
0aeb0ef
Add build flag back
amaury1093 Oct 7, 2020
917fc36
Merge branch 'am-7401-blockresults' of ssh://github.com/cosmos/cosmos…
amaury1093 Oct 7, 2020
2352678
Merge branch 'master' into am-7401-blockresults
mergify[bot] Oct 8, 2020
3e1a27b
More timeout on test
amaury1093 Oct 8, 2020
8ba50d0
Merge branch 'am-7401-blockresults' of ssh://github.com/cosmos/cosmos…
amaury1093 Oct 8, 2020
5fe0081
fix timeout-minutes
amaury1093 Oct 8, 2020
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
10 changes: 5 additions & 5 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ jobs:
name: "${{ github.sha }}-aa"
if: "env.GIT_DIFF != ''"
- name: Run tests with race detector
run: cat xaa.txt | xargs go test -mod=readonly -json -timeout 15m -race -tags='cgo ledger test_ledger_mock' > xaa-race-output.txt
run: cat xaa.txt | xargs go test -mod=readonly -json -timeout 30m -race -tags='cgo ledger test_ledger_mock' > xaa-race-output.txt
if: "env.GIT_DIFF != ''"
- uses: actions/upload-artifact@v2
with:
Expand Down Expand Up @@ -305,7 +305,7 @@ jobs:
name: "${{ github.sha }}-ab"
if: "env.GIT_DIFF != ''"
- name: Run tests with race detector
run: cat xab.txt | xargs go test -mod=readonly -json -timeout 15m -race -tags='cgo ledger test_ledger_mock' > xab-race-output.txt
run: cat xab.txt | xargs go test -mod=readonly -json -timeout 30m -race -tags='cgo ledger test_ledger_mock' > xab-race-output.txt
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alessio I'm doubling the timeout here, because:

Copy link
Member

Choose a reason for hiding this comment

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

There seems to be a test or something else that is consuming a massive amount time. Increasing the timeout to 30m defeats the purpose of splitting tests into multiple jobs (decrease overall time)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed 👍 . I see this more as a band-aid patch, as it's blocking ~3-4 automerge PRs. Unless someone has an idea for a quick fix?

Copy link
Member

@tac0turtle tac0turtle Oct 8, 2020

Choose a reason for hiding this comment

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

okay! 👍 . If its hanging then 30m wont change it

Copy link
Contributor Author

@amaury1093 amaury1093 Oct 8, 2020

Choose a reason for hiding this comment

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

my hypothesis is that it's not hanging, it passes in some PRs, the test I linked above finished in 14m39s.

but anyways, "Cancelled after 15m" https://github.com/cosmos/cosmos-sdk/pull/7442/checks?check_run_id=1225173890#step:7:1 :( Edit: fixed.

Copy link
Contributor

Choose a reason for hiding this comment

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

it's probably due to the IBC tests. We rely heavily on simapp since it's kinda hard to unit test interactions between different chains and using simapp makes testing a lot more robust. I noticed recently the test times on my machine almost doubled in duration, so I wonder if there was a change to simapp that slowed things down?

Copy link
Member

Choose a reason for hiding this comment

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

the -race flag slows things down a lot. This plus the IBC tests could be the culprit.

if: "env.GIT_DIFF != ''"
- uses: actions/upload-artifact@v2
with:
Expand Down Expand Up @@ -335,7 +335,7 @@ jobs:
name: "${{ github.sha }}-ac"
if: "env.GIT_DIFF != ''"
- name: Run tests with race detector
run: cat xac.txt | xargs go test -mod=readonly -json -timeout 15m -race -tags='cgo ledger test_ledger_mock' > xac-race-output.txt
run: cat xac.txt | xargs go test -mod=readonly -json -timeout 30m -race -tags='cgo ledger test_ledger_mock' > xac-race-output.txt
if: "env.GIT_DIFF != ''"
- uses: actions/upload-artifact@v2
with:
Expand Down Expand Up @@ -365,7 +365,7 @@ jobs:
name: "${{ github.sha }}-ad"
if: "env.GIT_DIFF != ''"
- name: Run tests with race detector
run: cat xad.txt | xargs go test -mod=readonly -json -timeout 15m -race -tags='cgo ledger test_ledger_mock' > xad-race-output.txt
run: cat xad.txt | xargs go test -mod=readonly -json -timeout 30m -race -tags='cgo ledger test_ledger_mock' > xad-race-output.txt
if: "env.GIT_DIFF != ''"
- uses: actions/upload-artifact@v2
with:
Expand Down Expand Up @@ -430,7 +430,7 @@ jobs:
run: |
./contrib/localnet_liveness.sh 100 5 50 localhost
if: "env.GIT_DIFF != ''"

docker-build:
runs-on: ubuntu-latest
timeout-minutes: 10
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ require (
github.com/tendermint/btcd v0.1.1
github.com/tendermint/crypto v0.0.0-20191022145703-50d29ede1e15
github.com/tendermint/go-amino v0.16.0
github.com/tendermint/tendermint v0.34.0-rc4
github.com/tendermint/tendermint v0.34.0-rc4.0.20201005135527-d7d0ffea13c6
github.com/tendermint/tm-db v0.6.2
golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a
google.golang.org/genproto v0.0.0-20200825200019-8632dd797987
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,8 @@ github.com/tendermint/tendermint v0.34.0-rc3 h1:d7Fsd5rdbxq4GmJ0kRfx7l7LesQM7e70
github.com/tendermint/tendermint v0.34.0-rc3/go.mod h1:BoHcEpjfpBHc1Be7RQz3AHaXFNObcDG7SNHCev6Or4g=
github.com/tendermint/tendermint v0.34.0-rc4 h1:fnPyDFz9QGAU6tjExoQ8ZY63eHkzdBg5StQgDoeuK0s=
github.com/tendermint/tendermint v0.34.0-rc4/go.mod h1:yotsojf2C1QBOw4dZrTcxbyxmPUrT4hNuOQWX9XUwB4=
github.com/tendermint/tendermint v0.34.0-rc4.0.20201005135527-d7d0ffea13c6 h1:gqZ0WDpDYgMm/iaiMEXvI1nt/GoWCuwtBomVpUMiAIs=
github.com/tendermint/tendermint v0.34.0-rc4.0.20201005135527-d7d0ffea13c6/go.mod h1:BSXqR6vWbOecet726v66qVwSkFDLfEeBrq+EhkKbij4=
github.com/tendermint/tm-db v0.6.1 h1:w3X87itMPXopcRPlFiqspEKhw4FXihPk2rnFFkP0zGk=
github.com/tendermint/tm-db v0.6.1/go.mod h1:m3x9kRP4UFd7JODJL0yBAZqE7wTw+S37uAE90cTx7OA=
github.com/tendermint/tm-db v0.6.2 h1:DOn8jwCdjJblrCFJbtonEIPD1IuJWpbRUUdR8GWE4RM=
Expand Down
20 changes: 12 additions & 8 deletions testutil/network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
srvconfig "github.com/cosmos/cosmos-sdk/server/config"
servertypes "github.com/cosmos/cosmos-sdk/server/types"
"github.com/cosmos/cosmos-sdk/simapp"
"github.com/cosmos/cosmos-sdk/simapp/params"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
Expand All @@ -52,13 +53,16 @@ var lock = new(sync.Mutex)
// creates an ABCI Application to provide to Tendermint.
type AppConstructor = func(val Validator) servertypes.Application

func NewSimApp(val Validator) servertypes.Application {
return simapp.NewSimApp(
val.Ctx.Logger, dbm.NewMemDB(), nil, true, make(map[int64]bool), val.Ctx.Config.RootDir, 0,
simapp.MakeEncodingConfig(),
baseapp.SetPruning(storetypes.NewPruningOptionsFromString(val.AppConfig.Pruning)),
baseapp.SetMinGasPrices(val.AppConfig.MinGasPrices),
)
// NewAppConstructor returns a new simapp AppConstructor
func NewAppConstructor(encodingCfg params.EncodingConfig) AppConstructor {
return func(val Validator) servertypes.Application {
return simapp.NewSimApp(
val.Ctx.Logger, dbm.NewMemDB(), nil, true, make(map[int64]bool), val.Ctx.Config.RootDir, 0,
encodingCfg,
baseapp.SetPruning(storetypes.NewPruningOptionsFromString(val.AppConfig.Pruning)),
baseapp.SetMinGasPrices(val.AppConfig.MinGasPrices),
)
}
Comment on lines +56 to +65
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just a refactor to avoid creating an additional simapp.MakeEncodingConfig()

}

// Config defines the necessary configuration used to bootstrap and start an
Expand Down Expand Up @@ -98,7 +102,7 @@ func DefaultConfig() Config {
LegacyAmino: encCfg.Amino,
InterfaceRegistry: encCfg.InterfaceRegistry,
AccountRetriever: authtypes.AccountRetriever{},
AppConstructor: NewSimApp,
AppConstructor: NewAppConstructor(encCfg),
GenesisState: simapp.ModuleBasics.DefaultGenesis(encCfg.Marshaler),
TimeoutCommit: 2 * time.Second,
ChainID: "chain-" + tmrand.NewRand().Str(6),
Expand Down
76 changes: 76 additions & 0 deletions x/staking/client/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cli_test

import (
"context"
json "encoding/json"
"fmt"
"strings"
Expand All @@ -11,6 +12,8 @@ import (
"github.com/gogo/protobuf/proto"
"github.com/stretchr/testify/suite"
tmcli "github.com/tendermint/tendermint/libs/cli"
"github.com/tendermint/tendermint/proto/tendermint/crypto"
"github.com/tendermint/tendermint/rpc/client/http"

"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/crypto/hd"
Expand All @@ -36,6 +39,10 @@ type IntegrationTestSuite struct {
func (s *IntegrationTestSuite) SetupSuite() {
s.T().Log("setting up integration test suite")

if testing.Short() {
s.T().Skip("skipping test in unit-tests mode.")
}

cfg := network.DefaultConfig()
cfg.NumValidators = 2

Expand Down Expand Up @@ -1260,6 +1267,75 @@ func (s *IntegrationTestSuite) TestNewCmdUnbond() {
}
}

// TestBlockResults tests that the validator updates correctly show when
// calling the /block_results RPC endpoint.
// ref: https://github.com/cosmos/cosmos-sdk/issues/7401.
func (s *IntegrationTestSuite) TestBlockResults() {
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
require := s.Require()
val := s.network.Validators[0]
robert-zaremba marked this conversation as resolved.
Show resolved Hide resolved

// Create new account in the keyring.
info, _, err := val.ClientCtx.Keyring.NewMnemonic("NewDelegator", keyring.English, sdk.FullFundraiserPath, hd.Secp256k1)
require.NoError(err)
newAddr := sdk.AccAddress(info.GetPubKey().Address())

// Send some funds to the new account.
_, err = banktestutil.MsgSendExec(
val.ClientCtx,
val.Address,
newAddr,
sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(200))), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
)
require.NoError(err)

// Use CLI to create a delegation from the new account to validator `val`.
delHeight, err := s.network.LatestHeight()
require.NoError(err)
cmd := cli.NewDelegateCmd()
_, err = clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, []string{
val.ValAddress.String(),
sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(150)).String(),
fmt.Sprintf("--%s=%s", flags.FlagFrom, newAddr.String()),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
})
require.NoError(err)

// Create a HTTP rpc client.
rpcClient, err := http.New(val.RPCAddress, "/websocket")

// Loop until we find a block result with the correct validator updates.
// By experience, it happens around 2 blocks after `delHeight`.
for {
latestHeight, err := s.network.LatestHeight()
require.NoError(err)

// Wait maximum 10 blocks, or else fail test.
if latestHeight > delHeight+10 {
s.Fail("timeout reached")
}

res, err := rpcClient.BlockResults(context.Background(), &latestHeight)
require.NoError(err)

if len(res.ValidatorUpdates) > 0 {
valUpdate := res.ValidatorUpdates[0]
require.Equal(
valUpdate.GetPubKey().Sum.(*crypto.PublicKey_Ed25519).Ed25519,
val.PubKey.Bytes(),
)

// We got our validator update, test passed.
break
}

s.network.WaitForNextBlock()
}
}

func TestIntegrationTestSuite(t *testing.T) {
suite.Run(t, new(IntegrationTestSuite))
}