Fix addGenesisAccount

[alexanderbez]

closes: #4271

---

• Targeted PR against correct branch (see CONTRIBUTING.md)

• Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

• Wrote tests

• Updated relevant documentation (docs/)

• Added a relevant changelog entry: clog add [section] [stanza] [message]

• rereviewed Files changed in the github PR explorer

---

For Admin Use:

• Added appropriate labels to PR (ex. wip, ready-for-review, docs)
• Reviewers Assigned
• Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

[codecov]

Codecov Report

Merging #4281 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4281      +/-   ##
==========================================
+ Coverage    59.1%   59.13%   +0.02%     
==========================================
  Files         217      217              
  Lines       14595    14603       +8     
==========================================
+ Hits         8627     8635       +8     
  Misses       5330     5330              
  Partials      638      638

[alessio]

ACKed. Thanks!

[alexanderbez]

Actually, @alessio @faboweb I reverted the suggested changes. It doesn't make sense to use AllLTE because you don't have to vest the full amount(s).

[alessio]

This should be

        // if len(coins) = 0 we know for sure no amount in coins can be GT coinsB
	if len(coins) == 0 {
		return false
	}

        // we learned from the above that coins contains some coins,
        // thus if len(coinsB) == 0 we know for sure there's something GT
	if len(coinsB) == 0 {
		return true
	}

[alexanderbez]

Again, it is important to be consistent across the methods. This behaves the same as IsAnyGTE. If coinsB then no denom is present and thus no coin is GT.

[alessio]

This:

	if len(coins) == 0 {
		return false
	}

adds a bit of extra readability (thanks to early return) and does not conflict with If coinsB then no denom is present and thus no coin is GT., in fact the for _, coin := range coins would just not be executed anyway when len(coins) == 0. Also it does not break consistency with IsAnyGTE.

Though I see your point, this should be dropped:

	if len(coinsB) == 0 {
		return true
	}

[alexanderbez]

Ok, I'll update.

[alexanderbez]

Actually,

if len(coins) == 0 {
		return false
	}

is redundant though.

[alessio]

I said so above :)

in fact the for _, coin := range coins would just not be executed anyway when len(coins) == 0

It's just early return, readability is added by sparing the reader from continuing reading the rest of the function when len(coins) == 0

[alexanderbez]

tbh I don't think this is necessary at all. The method is pretty concise as-is.

[rigelrozanski]

nice

[fedekunze]

LGTM