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

Non-Deterministic Bonding Test #3559

Closed
4 tasks
alexanderbez opened this issue Feb 8, 2019 · 11 comments · Fixed by #3710
Closed
4 tasks

Non-Deterministic Bonding Test #3559

alexanderbez opened this issue Feb 8, 2019 · 11 comments · Fixed by #3710

Comments

@alexanderbez
Copy link
Contributor

PRs recently targeted against develop, have been failing intermittently on the following test:

--- FAIL: TestBonding (4.77s)
	Error Trace:	lcd_test.go:644
	Error:      	Not equal: 
	            	expected: 30000000.000000000000000000
	            	actual  : 30303030.303030303030303030
	            	
	            	Diff:
	            	--- Expected
	            	+++ Actual
	            	@@ -1,2 +1,2 @@
	            	-(types.Dec) 30000000.000000000000000000
	            	+(types.Dec) 30303030.303030303030303030
	            	 
	Test:       	TestBonding

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor Author

May be addressed by #3553 -- related to minting.

@cwgoes
Copy link
Contributor

cwgoes commented Feb 8, 2019

Likely the issue is that the Tendermint instance is running for a non-constant number of blocks before we query it.

If you set the initial inflation, min inflation, and max inflation to zero there should be no inflation.

And I don't think we can rely on query results to return the exact inflation if we don't precisely specify the number of blocks we're running for in an LCD test.

@alessio
Copy link
Contributor

alessio commented Feb 8, 2019

@cwgoes:

	inflationMin := sdk.ZeroDec()
	if minting {
		inflationMin = sdk.MustNewDecFromStr("10000.0")
		genesisState.MintData.Params.InflationMax = sdk.MustNewDecFromStr("15000.0")
	} else {
		genesisState.MintData.Params.InflationMax = inflationMin
	}
	genesisState.MintData.Minter.Inflation = inflationMin
	genesisState.MintData.Params.InflationMin = inflationMin

alessio pushed a commit that referenced this issue Feb 8, 2019
- Also fix flaky test (closes: #3559). Don't test values
  returned by queries since there's no way to query a
  specific height via REST.
@cwgoes cwgoes closed this as completed in e7e3c32 Feb 8, 2019
@alessio
Copy link
Contributor

alessio commented Feb 12, 2019

The dog is still barking a hell of a lot. Hence reopening.

@alessio alessio reopened this Feb 12, 2019
@alessio alessio added this to the v0.32.0 (Enable Transfers) milestone Feb 12, 2019
@jackzampolin
Copy link
Member

Looks like this was fixed. Closing

@cwgoes cwgoes reopened this Feb 12, 2019
@cwgoes
Copy link
Contributor

cwgoes commented Feb 12, 2019

(fa2e9b8 fixed another failure, not this one)

@rigelrozanski
Copy link
Contributor

The easy suspect is tokens being minted due to a delay in the timing... has anyone been able to replicate this consistently with some time.Sleep('s placed around?

@alexanderbez
Copy link
Contributor Author

The easy suspect is tokens being minted due to a delay in the timing... has anyone been able to replicate this consistently with some time.Sleep('s placed around?

@rigelrozanski, @alessio stated the above happens even when inflation is turned off.

@alessio
Copy link
Contributor

alessio commented Feb 13, 2019

Correct @alexanderbez.
In TestBonding, fixtures are initiliazed with 0 inflation (as in InitializeTestLCD call's trailing argument is false):

func TestBonding(t *testing.T) {
	kb, err := keys.NewKeyBaseFromDir(InitClientHome(t, ""))
	require.NoError(t, err)
	addr, _ := CreateAddr(t, name1, pw, kb)

	cleanup, valPubKeys, operAddrs, port := InitializeTestLCD(t, 2, []sdk.AccAddress{addr}, false)

@rigelrozanski
Copy link
Contributor

madness

@rigelrozanski
Copy link
Contributor

rigelrozanski commented Feb 22, 2019

So this would actually make sense - this delegation that's we're querying to is the delegation created to the second validator (which does not sign in the lcd tests - hence eventually it should get slashed)... if the test takes long enough and the second validator gets slashed before this redelegation takes place then when the redegation does take place it the shares it receives will be greater than 1-to-1 (because the existing original shares of the second validator are worth less than 1 due to the slash)...

Thus the mystery of where these extra shares come from makes sense... the solution is to simply query the exchange rate of the second validator and verify tokens instead of shares

:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants