feat: TotalCoin and TotalCoinKeeper implementation #2671
Conversation
implementation of totalCoin --------- Co-authored-by: ThinhNX <168700277+thinhnx-var@users.noreply.github.com>
thinhnx-var
requested review from
jaekwon,
deelawn,
thehowl,
mvertes,
moul,
gfanton,
zivkovicmilos and
piux2
as code owners
github-actions
bot
added
📦 🤖 gnovm
thinhnx-var
changed the title
|
@leohhhn |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2671 +/- ##
==========================================
+ Coverage 60.44% 60.45% +0.01%
==========================================
Files 563 564 +1
Lines 75159 75224 +65
==========================================
+ Hits 45431 45478 +47
- Misses 26341 26352 +11
- Partials 3387 3394 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
I like this approach because we don't have to iterate through all accounts every time we query the |
|
LGTM. |
…ithub.com/VAR-META-Tech/gno into dev-thinhnx/feat_gno_implement_total_coin
hey @ltzmaxwell , the CI is failing because some of my additions are not yet covered by tests. I will probably handle this after #2319 is merged. As for the functionality, I think it's ready for review |
| @@ -128,15 +130,18 @@ func (bank BankKeeper) SubtractCoins(ctx sdk.Context, addr crypto.Address, amt s | |||
| } | |||
There was a problem hiding this comment.
if acc not exist, we can halt here, right?
There was a problem hiding this comment.
that's right, this's an exception that shouldn't happen
gnovm/tests/file.go
Outdated
| } | ||
|
|
||
| func (tb *testBanker) IssueCoin(addr crypto.Bech32Address, denom string, amt int64) { | ||
| coins, _ := tb.coinTable[addr] | ||
| sum := coins.Add(std.Coins{{denom, amt}}) | ||
| tb.coinTable[addr] = sum | ||
| tb.totalCoin[denom] += amt |
There was a problem hiding this comment.
do we need to check overflow/underflow, as what coins.Add/coins.Sub already did.
gnovm/tests/file.go
Outdated
|
|
||
| totalCoin, ok := overflow.Sub64(tb.totalCoin[denom], amt) | ||
| if !ok { | ||
| panic(fmt.Sprintf("totalCoin overflow/underflow for denom %s while removing %d", denom, amt)) |
There was a problem hiding this comment.
Nitpick: here we can already distinguish whether it's overflow or underflow, right?
There was a problem hiding this comment.
There was a problem hiding this comment.
the prev functions i used actually didn't bring panic info
Contributors' checklist...
BREAKING CHANGE: xxxmessage was included in the descriptionImplementation of #2662
TotalCoinKeeper