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.

Sign up for GitHub

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

services/horizon: Correctly check trustline flags to classify asset balances #3847

Merged
merged 4 commits into from
Aug 26, 2021
Merged

services/horizon: Correctly check trustline flags to classify asset balances #3847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3a6649d
use method accessors for asset stats amount classification
Aug 25, 2021
6bad238
Update changelog
Aug 25, 2021
63916f2
bump ingest.CurrentVersion
Aug 25, 2021
d9c22ca
Fix up test Post rebase
Aug 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions services/horizon/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
All notable changes to this project will be documented in this
file. This project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased

* Fix bug in asset balance classification where clawback is enabled. ([3847](https://github.com/stellar/go/pull/3847))

## v2.8.0
**Upgrading to this version from <= v2.1.1 will trigger a state rebuild. During this process (which will take at least 10 minutes), Horizon will not ingest new ledgers.**

Expand Down
3 changes: 2 additions & 1 deletion services/horizon/internal/ingest/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ const (
// in ClaimableBalances predicates.
// - 13: Trigger state rebuild to include more than just authorized assets.
// - 14: Trigger state rebuild to include claimable balances in the asset stats processor.
CurrentVersion = 14
// - 15: Fixed bug in asset stat ingestion where clawback is enabled (#3846).
CurrentVersion = 15

// MaxDBConnections is the size of the postgres connection pool dedicated to Horizon ingestion:
// * Ledger ingestion,
Expand Down
39 changes: 39 additions & 0 deletions services/horizon/internal/ingest/processors/asset_stats_processor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,45 @@ func (s *AssetStatsProcessorTestSuiteState) TestCreateTrustLine() {
}, maxBatchSize).Return(nil).Once()
}

func (s *AssetStatsProcessorTestSuiteState) TestCreateTrustLineWithClawback() {
trustLine := xdr.TrustLineEntry{
AccountId: xdr.MustAddress("GAOQJGUAB7NI7K7I62ORBXMN3J4SSWQUQ7FOEPSDJ322W2HMCNWPHXFB"),
Asset: xdr.MustNewCreditAsset("EUR", trustLineIssuer.Address()),
Flags: xdr.Uint32(xdr.TrustLineFlagsAuthorizedFlag | xdr.TrustLineFlagsTrustlineClawbackEnabledFlag),
}
lastModifiedLedgerSeq := xdr.Uint32(123)

err := s.processor.ProcessChange(s.ctx, ingest.Change{
Type: xdr.LedgerEntryTypeTrustline,
Pre: nil,
Post: &xdr.LedgerEntry{
Data: xdr.LedgerEntryData{
Type: xdr.LedgerEntryTypeTrustline,
TrustLine: &trustLine,
},
LastModifiedLedgerSeq: lastModifiedLedgerSeq,
},
})
s.Assert().NoError(err)

s.mockQ.On("InsertAssetStats", s.ctx, []history.ExpAssetStat{
{
AssetType: xdr.AssetTypeAssetTypeCreditAlphanum4,
AssetIssuer: trustLineIssuer.Address(),
AssetCode: "EUR",
Accounts: history.ExpAssetStatAccounts{Authorized: 1},
Balances: history.ExpAssetStatBalances{
Authorized: "0",
AuthorizedToMaintainLiabilities: "0",
Unauthorized: "0",
ClaimableBalances: "0",
},
Amount: "0",
NumAccounts: 1,
},
}, maxBatchSize).Return(nil).Once()
}

func (s *AssetStatsProcessorTestSuiteState) TestCreateTrustLineUnauthorized() {
trustLine := xdr.TrustLineEntry{
AccountId: xdr.MustAddress("GAOQJGUAB7NI7K7I62ORBXMN3J4SSWQUQ7FOEPSDJ322W2HMCNWPHXFB"),
Expand Down
8 changes: 4 additions & 4 deletions services/horizon/internal/ingest/processors/asset_stats_set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,12 @@ type delta struct {
}

func (d *delta) addByFlags(flags xdr.Uint32, amount int64) {
switch xdr.TrustLineFlags(flags) {
case xdr.TrustLineFlagsAuthorizedFlag:
f := xdr.TrustLineFlags(flags)
if f.IsAuthorized() {
paulbellamy marked this conversation as resolved.
Show resolved Hide resolved
d.Authorized += amount
case xdr.TrustLineFlagsAuthorizedToMaintainLiabilitiesFlag:
} else if f.IsAuthorizedToMaintainLiabilitiesFlag() {
d.AuthorizedToMaintainLiabilities += amount
default:
} else {
d.Unauthorized += amount
}
}
Expand Down
2 changes: 1 addition & 1 deletion services/horizon/internal/ingest/processors/asset_stats_set_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func TestAddAndRemoveAssetStats(t *testing.T) {
AccountId: xdr.MustAddress("GAOQJGUAB7NI7K7I62ORBXMN3J4SSWQUQ7FOEPSDJ322W2HMCNWPHXFB"),
Asset: xdr.MustNewCreditAsset(eur, trustLineIssuer.Address()),
Balance: 24,
Flags: xdr.Uint32(xdr.TrustLineFlagsAuthorizedFlag),
Flags: xdr.Uint32(xdr.TrustLineFlagsAuthorizedFlag | xdr.TrustLineFlagsTrustlineClawbackEnabledFlag),
})),
)

Expand Down
2 changes: 1 addition & 1 deletion services/horizon/internal/ingest/verify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const assetStatsBatchSize = 500
// check them.
// There is a test that checks it, to fix it: update the actual `verifyState`
// method instead of just updating this value!
const stateVerifierExpectedIngestionVersion = 14
const stateVerifierExpectedIngestionVersion = 15

// verifyState is called as a go routine from pipeline post hook every 64
// ledgers. It checks if the state is correct. If another go routine is already
Expand Down