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

All vesting coins' OriginalVesting amount should LessEqual to acc.Coins #4271

Closed
1 of 4 tasks
whunmr opened this issue May 5, 2019 · 1 comment · Fixed by #4281
Closed
1 of 4 tasks

All vesting coins' OriginalVesting amount should LessEqual to acc.Coins #4271

whunmr opened this issue May 5, 2019 · 1 comment · Fixed by #4281
Labels
C:genesis relating to chain genesis T:Bug

Comments

@whunmr
Copy link

whunmr commented May 5, 2019

Summary of Bug

Should use bvacc.OriginalVesting.IsAnyGT(acc.Coins) instead of IsAllGT.

code link

if bvacc.OriginalVesting.IsAllGT(acc.Coins) {
			return appState, fmt.Errorf("vesting amount cannot be greater than total amount")
}

Version

v0.34.3

Steps to Reproduce

in following test:

    --- PASS: TestAddGenesisAccount/invalid_vesting_amount (0.00s)
    --- FAIL: TestAddGenesisAccount/invalid_vesting_amount_with_multi_coins (0.00s)
        require.go:157: 
            	Error Trace:	genesis_accts_test.go:61
            	Error:      	Not equal: 
            	            	expected: true
            	            	actual  : false
            	Test:       	TestAddGenesisAccount/invalid_vesting_amount_with_multi_coin
func TestAddGenesisAccount(t *testing.T) {
	cdc := codec.New()
	addr1 := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address())
	type args struct {
		appState     app.GenesisState
		addr         sdk.AccAddress
		coins        sdk.Coins
		vestingAmt   sdk.Coins
		vestingStart int64
		vestingEnd   int64
	}
	tests := []struct {
		name    string
		args    args
		wantErr bool
	}{
		{
			"invalid vesting amount",
			args{
				app.GenesisState{},
				addr1,
				sdk.NewCoins(sdk.NewInt64Coin("uatom", 50)),
				sdk.NewCoins(sdk.NewInt64Coin("uatom", 100)),
				0,
				1,
			},
			true,
		},
		{
			"invalid vesting amount with multi coins",
			args{
				app.GenesisState{},
				addr1,
				sdk.NewCoins(sdk.NewInt64Coin("uatom", 50), sdk.NewInt64Coin("aaa", 50)),
				sdk.NewCoins(sdk.NewInt64Coin("uatom", 100), sdk.NewInt64Coin("aaa", 20)),
				0,
				1,
			},
			true,
		},
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			_, err := addGenesisAccount(
				cdc, tt.args.appState, tt.args.addr, tt.args.coins,
				tt.args.vestingAmt, tt.args.vestingStart, tt.args.vestingEnd,
			)
			require.Equal(t, tt.wantErr, (err != nil))
		})
	}
}


For Admin Use

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

Thanks for opening up an issue @whunmr. Indeed this seems problematic during genesis. Thankfully the hub is only a single coin so this is not problematic.

@alexanderbez alexanderbez added T:Bug C:genesis relating to chain genesis labels May 6, 2019
@alexanderbez alexanderbez mentioned this issue May 6, 2019
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:genesis relating to chain genesis T:Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants