Skip to content

Commit

Permalink
Fix sum of asset in circulation in all assets endpoint
Browse files Browse the repository at this point in the history
Currently we hold sum of asset in circulation in `int64` variable. Given
that issuer can send `MAX_INT64` of an asset to each of it's trustees
it's very easy to overflow the type.

This commit changes the type a variable that holds DB result for a query
to `string` and adds a new function in `amount` package to convert
string integer to Stellar amount.
  • Loading branch information
bartekn committed May 7, 2018
1 parent 670a065 commit 4cbce8a
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 21 deletions.
15 changes: 15 additions & 0 deletions amount/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,21 @@ func ParseInt64(v string) (int64, error) {
return i, nil
}

// IntStringToAmount converts string integer value and converts it to stellar
// "amount". In other words, it divides the given string integer value by 10^7
// and returns the string representation of that number.
// It is safe to use with values exceeding int64 limits.
func IntStringToAmount(v string) (string, error) {
r := &big.Rat{}
if _, ok := r.SetString(v); !ok {
return "", errors.Errorf("cannot parse amount: %s", v)
}

r.Quo(r, bigOne)

return r.FloatString(7), nil
}

// String returns an "amount string" from the provided raw xdr.Int64 value `v`.
func String(v xdr.Int64) string {
return StringFromInt64(int64(v))
Expand Down
43 changes: 43 additions & 0 deletions amount/main_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package amount_test

import (
"fmt"
"testing"

"github.com/stellar/go/amount"
Expand Down Expand Up @@ -58,3 +59,45 @@ func TestString(t *testing.T) {
}
}
}

func TestIntStringToAmount(t *testing.T) {
var testCases = []struct {
Output string
Input string
Valid bool
}{
{"100.0000000", "1000000000", true},
{"-100.0000000", "-1000000000", true},
{"100.0000001", "1000000001", true},
{"123.0000001", "1230000001", true},
{"922337203685.4775807", "9223372036854775807", true},
{"922337203685.4775808", "9223372036854775808", true},
{"92233.7203686", "922337203686", true},
{"-922337203685.4775808", "-9223372036854775808", true},
{"-922337203685.4775809", "-9223372036854775809", true},
{"-92233.7203686", "-922337203686", true},
{"1000000000000.0000000", "10000000000000000000", true},
{"0.0000000", "0", true},
{"", "nan", false},
}

for _, tc := range testCases {
t.Run(fmt.Sprintf("%s to %s (valid = %t)", tc.Input, tc.Output, tc.Valid), func(t *testing.T) {
o, err := amount.IntStringToAmount(tc.Input)

if !tc.Valid && err == nil {
t.Errorf("expected err for input %s (output: %s)", tc.Input, tc.Output)
return
}
if tc.Valid && err != nil {
t.Errorf("couldn't parse %s: %v", tc.Input, err)
return
}

if o != tc.Output {
t.Errorf("%s converted to %s, not %s", tc.Input, o, tc.Output)
}
})
}

}
2 changes: 1 addition & 1 deletion services/horizon/internal/db2/assets/asset_stat.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type AssetStatsR struct {
Type string `db:"asset_type"`
Code string `db:"asset_code"`
Issuer string `db:"asset_issuer"`
Amount int64 `db:"amount"`
Amount string `db:"amount"`
NumAccounts int32 `db:"num_accounts"`
Flags int8 `db:"flags"`
Toml string `db:"toml"`
Expand Down
6 changes: 3 additions & 3 deletions services/horizon/internal/db2/assets/asset_stat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func TestAssetsStatsQExec(t *testing.T) {
Type: "credit_alphanum4",
Code: "BTC",
Issuer: "GC23QF2HUE52AMXUFUH3AYJAXXGXXV2VHXYYR6EYXETPKDXZSAW67XO4",
Amount: 1009876000,
Amount: "1009876000",
NumAccounts: 1,
Flags: 1,
Toml: "https://test.com/.well-known/stellar.toml",
Expand All @@ -26,7 +26,7 @@ func TestAssetsStatsQExec(t *testing.T) {
Type: "credit_alphanum4",
Code: "SCOT",
Issuer: "GCXKG6RN4ONIEPCMNFB732A436Z5PNDSRLGWK7GBLCMQLIFO4S7EYWVU",
Amount: 10000000000,
Amount: "10000000000",
NumAccounts: 1,
Flags: 2,
Toml: "",
Expand All @@ -37,7 +37,7 @@ func TestAssetsStatsQExec(t *testing.T) {
Type: "credit_alphanum4",
Code: "USD",
Issuer: "GC23QF2HUE52AMXUFUH3AYJAXXGXXV2VHXYYR6EYXETPKDXZSAW67XO4",
Amount: 3000010434000,
Amount: "3000010434000",
NumAccounts: 2,
Flags: 1,
Toml: "https://test.com/.well-known/stellar.toml",
Expand Down
6 changes: 3 additions & 3 deletions services/horizon/internal/db2/core/trustline.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,16 @@ func (q *Q) BalancesForAsset(
assetType int32,
assetCode string,
assetIssuer string,
) (int32, int64, error) {
) (int32, string, error) {
sql := selectBalances.Where(sq.Eq{
"assettype": assetType,
"assetcode": assetCode,
"issuer": assetIssuer,
"flags": 1,
})
result := struct {
Count int32 `db:"count"`
Sum int64 `db:"sum"`
Count int32 `db:"count"`
Sum string `db:"sum"`
}{}
err := q.Get(&result, sql)
return result.Count, result.Sum, err
Expand Down
2 changes: 1 addition & 1 deletion services/horizon/internal/db2/history/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ type Asset struct {
// AssetStat is a row in the asset_stats table representing the stats per Asset
type AssetStat struct {
ID int64 `db:"id"`
Amount int64 `db:"amount"`
Amount string `db:"amount"`
NumAccounts int32 `db:"num_accounts"`
Flags int8 `db:"flags"`
Toml string `db:"toml"`
Expand Down
2 changes: 1 addition & 1 deletion services/horizon/internal/ingest/asset_stat.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func computeAssetStat(is *Session, asset *xdr.Asset) *history.AssetStat {
}

// statTrustlinesInfo fetches all the stats from the trustlines table
func statTrustlinesInfo(coreQ *core.Q, assetType xdr.AssetType, assetCode string, assetIssuer string) (int32, int64, error) {
func statTrustlinesInfo(coreQ *core.Q, assetType xdr.AssetType, assetCode string, assetIssuer string) (int32, string, error) {
return coreQ.BalancesForAsset(int32(assetType), assetCode, assetIssuer)
}

Expand Down
22 changes: 11 additions & 11 deletions services/horizon/internal/ingest/asset_stat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestStatTrustlinesInfo(t *testing.T) {
assetCode string
assetIssuer string
wantNumAccounts int32
wantAmount int64
wantAmount string
}

testCases := []struct {
Expand All @@ -33,7 +33,7 @@ func TestStatTrustlinesInfo(t *testing.T) {
"USD",
"GC23QF2HUE52AMXUFUH3AYJAXXGXXV2VHXYYR6EYXETPKDXZSAW67XO4",
1,
0,
"0",
}},
}, {
"asset_stat_trustlines_2",
Expand All @@ -42,7 +42,7 @@ func TestStatTrustlinesInfo(t *testing.T) {
"USD",
"GC23QF2HUE52AMXUFUH3AYJAXXGXXV2VHXYYR6EYXETPKDXZSAW67XO4",
1,
0,
"0",
}},
}, {
"asset_stat_trustlines_3",
Expand All @@ -51,13 +51,13 @@ func TestStatTrustlinesInfo(t *testing.T) {
"USD1",
"GC23QF2HUE52AMXUFUH3AYJAXXGXXV2VHXYYR6EYXETPKDXZSAW67XO4",
1,
0,
"0",
}, {
xdr.AssetTypeAssetTypeCreditAlphanum4,
"USD2",
"GC23QF2HUE52AMXUFUH3AYJAXXGXXV2VHXYYR6EYXETPKDXZSAW67XO4",
1,
0,
"0",
}},
}, {
"asset_stat_trustlines_4",
Expand All @@ -66,13 +66,13 @@ func TestStatTrustlinesInfo(t *testing.T) {
"USD",
"GC23QF2HUE52AMXUFUH3AYJAXXGXXV2VHXYYR6EYXETPKDXZSAW67XO4",
1,
0,
"0",
}, {
xdr.AssetTypeAssetTypeCreditAlphanum4,
"USD",
"GCXKG6RN4ONIEPCMNFB732A436Z5PNDSRLGWK7GBLCMQLIFO4S7EYWVU",
1,
0,
"0",
}},
}, {
"asset_stat_trustlines_5",
Expand All @@ -81,7 +81,7 @@ func TestStatTrustlinesInfo(t *testing.T) {
"USD",
"GC23QF2HUE52AMXUFUH3AYJAXXGXXV2VHXYYR6EYXETPKDXZSAW67XO4",
1,
0,
"0",
}},
}, {
"asset_stat_trustlines_6",
Expand All @@ -90,7 +90,7 @@ func TestStatTrustlinesInfo(t *testing.T) {
"USD",
"GC23QF2HUE52AMXUFUH3AYJAXXGXXV2VHXYYR6EYXETPKDXZSAW67XO4",
1,
1012345000,
"1012345000",
}},
}, {
"asset_stat_trustlines_7",
Expand All @@ -99,7 +99,7 @@ func TestStatTrustlinesInfo(t *testing.T) {
"USD",
"GC23QF2HUE52AMXUFUH3AYJAXXGXXV2VHXYYR6EYXETPKDXZSAW67XO4",
2,
1012345000,
"1012345000",
}},
}, {
"allow_trust",
Expand All @@ -108,7 +108,7 @@ func TestStatTrustlinesInfo(t *testing.T) {
"USD",
"GC23QF2HUE52AMXUFUH3AYJAXXGXXV2VHXYYR6EYXETPKDXZSAW67XO4",
1, // assets with the auth_required flag should only be counted for authorized accounts
0,
"0",
}},
},
}
Expand Down
5 changes: 4 additions & 1 deletion services/horizon/internal/resource/asset_stat.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ func (res *AssetStat) Populate(
res.Asset.Type = row.Type
res.Asset.Code = row.Code
res.Asset.Issuer = row.Issuer
res.Amount = amount.StringFromInt64(row.Amount)
res.Amount, err = amount.IntStringToAmount(row.Amount)
if err != nil {
return err
}
res.NumAccounts = row.NumAccounts
res.Flags = AccountFlags{
(row.Flags & int8(xdr.AccountFlagsAuthRequiredFlag)) != 0,
Expand Down

0 comments on commit 4cbce8a

Please sign in to comment.