fix(star-gifts): refuse upgrades of deleted gifts and align withdrawal docs

A deleted gift keeps its row, and a user message ref still resolves to it
by owner and msg_id. The paid upgrade path checked only Converted and
UniqueGiftID, so an upgrade of a deleted, upgradable gift ran until the
upgrade_msg_id write found no aggregate row and failed with an internal
error. The transaction rolled back, but the client got INTERNAL instead
of STARGIFT_INVALID.

The upgrade form now refuses a deleted gift before issuing a payment
form, and the upgrade transaction re-checks the lifecycle under the row
lock. TestStarGiftDeleteBlocksLaterUpgradePostgres pins that no Stars
are charged and nothing is minted.

docs/collectible-gifts.md described tmp_password as unsupported; main
issues account.getTmpPassword grants and serves exact Layer 225-231 via
payments.getStarGiftWithdrawalUrlLegacy with SRP.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
iamxvbaba
2026-10-24 19:39:22 +08:00
Unverified
parent 9f23fd0989
commit ac81425363
6 changed files with 151 additions and 17 deletions
+1 -1
View File
@@ -64,7 +64,7 @@ jobs:
- name: Verify gift inventory and Stars ledger against PostgreSQL
env:
TELESRV_TEST_POSTGRES_DSN: postgres://inventory:inventory-test@127.0.0.1:5432/gift_inventory_test?sslmode=disable
run: go test ./internal/store/postgres -run '^(TestOfficialStarGiftInventoryPostgres|TestOfficialStarGiftBundleIsAtomicPostgres|TestStarGiftStorePostgres|TestStarsLedgerPostgres|TestBotGiftPurchaseIsNotConvertibleToStarsPostgres|TestStarGiftDeleteRemovesGiftFromProfilePostgres|TestStarGiftDeleteRefusesUpgradedGiftPostgres|TestStarGiftDeleteRefreshesCollectionHashPostgres|TestStarGiftDeleteRefusesIrreversibleGiftsPostgres)$' -count=1 -timeout=3m
run: go test ./internal/store/postgres -run '^(TestOfficialStarGiftInventoryPostgres|TestOfficialStarGiftBundleIsAtomicPostgres|TestStarGiftStorePostgres|TestStarsLedgerPostgres|TestBotGiftPurchaseIsNotConvertibleToStarsPostgres|TestStarGiftDeleteRemovesGiftFromProfilePostgres|TestStarGiftDeleteRefusesUpgradedGiftPostgres|TestStarGiftDeleteRefreshesCollectionHashPostgres|TestStarGiftDeleteRefusesIrreversibleGiftsPostgres|TestStarGiftDeleteBlocksLaterUpgradePostgres)$' -count=1 -timeout=3m
- name: Verify admin operator and audit command flow against PostgreSQL
env:
+16 -15
View File
@@ -68,21 +68,19 @@ Preconditions are checked in this order, because each later one would otherwise
reveal state to a caller who has not proven who they are:
1. Known method, non-bot caller (`BOT_METHOD_INVALID`).
2. Layer 232 replaced the SRP challenge with a temporary password: the request
carries `tmp_password:bytes` instead of `inputCheckPasswordSRP`. This server
issues no temporary passwords (there is no `account.getTmpPassword` surface),
so a non-empty `tmp_password` cannot be verified and is refused with
`PASSWORD_INVALID`. An empty `tmp_password` keeps the historical
"no password supplied" semantics: `checkSRP` accepts the empty check only on
accounts without a password, and only a real SRP answer on accounts with
one. Wrong answer → `PASSWORD_HASH_INVALID`, stale `srp_id` →
`SRP_ID_INVALID`.
Historical profiles (Layer 225–231) still send the SRP challenge; the
profile-to-canonical conversion discards it instead of promoting it into a
temporary password, so those clients reach the same empty-password check.
3. Freshness, only when the account has a password: `PASSWORD_TOO_FRESH_<wait>`
for a recently changed password and `SESSION_TOO_FRESH_<wait>` for a session
that is younger than the same window (`revenueWithdrawalFreshWait`).
2. The account must have a 2FA password; without one there is nothing to
prove and the call fails with `PASSWORD_MISSING`.
3. The 2FA proof. Layer 232 carries `tmp_password:bytes`: it must be the grant
that `account.getTmpPassword` issued to this authorization, still within
its `valid_until`, on the same login and under the current 2FA password;
anything else, empty included, is `TMP_PASSWORD_INVALID`. There is no SRP
fallback. Exact Layer 225–231 clients call
`payments.getStarGiftWithdrawalUrlLegacy` with the SRP challenge, and only
`inputCheckPasswordSRP` is accepted there. Wrong answer →
`PASSWORD_HASH_INVALID`, stale `srp_id` → `SRP_ID_INVALID`.
Freshness follows the proof: `PASSWORD_TOO_FRESH_<wait>` for a recently
changed password and `SESSION_TOO_FRESH_<wait>` for a session that is
younger than the same window (`revenueWithdrawalFreshWait`).
4. The auth key exists, belongs to the caller, and is not `password_pending`
(`AUTH_KEY_UNREGISTERED`).
5. The gift ref resolves and the caller is its owner (`STARGIFT_INVALID`,
@@ -141,6 +139,9 @@ Bool`. The owner removes a received gift from the profile.
- Refused states (`STARGIFT_INVALID`): converted, exported, burned, already
deleted, upgraded into a collectible, limited/auction release, still
convertible or prepaid.
- A deleted gift cannot be upgraded afterwards either: the upgrade form refuses
it (`STARGIFT_INVALID`) and the upgrade transaction re-checks the lifecycle
under the row lock, so no Stars are charged and no collectible is minted.
- The gift is dropped from every collection it was part of under the owner
collection advisory lock, the affected collection hashes are refreshed (so
`payments.getStarGiftCollections` cannot answer `NotModified` for a changed
@@ -100,3 +100,30 @@ func TestPaymentsDeleteStarGiftMapsStoreFailure(t *testing.T) {
t.Fatalf("owner err = %v, want STARGIFT_OWNER_INVALID", err)
}
}
// deletedStarGiftRPCGifts resolves the ref to a gift that was already deleted.
type deletedStarGiftRPCGifts struct {
GiftsService
owner int64
}
func (s *deletedStarGiftRPCGifts) GetSaved(_ context.Context, ref domain.SavedStarGiftRef) (domain.SavedStarGift, bool, error) {
return domain.SavedStarGift{
ID: 11, Owner: domain.Peer{Type: domain.PeerTypeUser, ID: s.owner}, MsgID: ref.MsgID, FromUserID: s.owner + 1,
GiftID: 5, LifecycleStatus: domain.StarGiftLifecycleDeleted, Unsaved: true,
}, true, nil
}
func TestStarGiftUpgradeFormRefusesDeletedGift(t *testing.T) {
r, sender, _, _ := starGiftTestRouter(t)
r.deps.Gifts = &deletedStarGiftRPCGifts{owner: sender.ID}
ctx := WithUserID(context.Background(), sender.ID)
// The preview is never consulted: the embedded nil GiftsService would panic.
_, err := r.starGiftUpgradePaymentForm(ctx, sender.ID, &tg.InputInvoiceStarGiftUpgrade{
Stargift: &tg.InputSavedStarGiftUser{MsgID: 4242},
})
if !tgerr.Is(err, "STARGIFT_INVALID") {
t.Fatalf("upgrade form for a deleted gift err = %v, want STARGIFT_INVALID", err)
}
}
+4 -1
View File
@@ -139,7 +139,10 @@ func (r *Router) starGiftUpgradeSavedTarget(ctx context.Context, userID int64, i
if err != nil {
return domain.SavedStarGift{}, internalErr()
}
if !found {
// A deleted gift was never upgraded (payments.deleteStarGift refuses
// collectibles), so no upgrade receipt can be replayed for it: refuse it
// before a payment form is issued.
if !found || saved.LifecycleStatus == domain.StarGiftLifecycleDeleted {
return domain.SavedStarGift{}, starGiftInvalidErr()
}
return saved, nil
@@ -156,6 +156,104 @@ func TestStarGiftDeleteRefusesUpgradedGiftPostgres(t *testing.T) {
}
}
// TestStarGiftDeleteBlocksLaterUpgradePostgres pins the other direction: a
// deleted gift keeps its row, but a paid upgrade must refuse it under the row
// lock instead of charging Stars against a gift that left the profile.
func TestStarGiftDeleteBlocksLaterUpgradePostgres(t *testing.T) {
pool := testPool(t)
ctx := context.Background()
suffix := randomSuffix(t)
now := int(time.Now().Unix())
users := NewUserStore(pool)
owner := createTestUser(t, ctx, users, "+1887"+suffix+"01", "DeletedOwner", "")
sender := createTestUser(t, ctx, users, "+1887"+suffix+"02", "DeletedSender", "")
ownerPeer := domain.Peer{Type: domain.PeerTypeUser, ID: owner.ID}
gifts := NewStarGiftStore(pool)
baseDocumentID := time.Now().UnixNano() & 0x7ffffffffffff000
entry, err := gifts.CreateCatalogRevision(ctx, domain.StarGiftCatalogWrite{
Title: "Delete upgrade " + suffix, Stars: 50, ConvertStars: 0, Enabled: true,
Document: collectibleTestDocument(baseDocumentID, "delete-upgrade.tgs"),
Blob: collectibleTestBlob(baseDocumentID, "delete-upgrade"), Animation: collectibleTestAnimation("delete-upgrade.tgs"),
Actor: "integration", CommandID: "delete-upgrade-catalog-" + suffix,
})
if err != nil {
t.Fatalf("create delete catalog: %v", err)
}
if _, err := gifts.PublishCollectibleRevision(ctx, domain.StarGiftCollectibleWrite{
GiftID: entry.Gift.ID, UpgradeStars: 100, SupplyTotal: 5, SlugPrefix: "delup-" + suffix,
Models: []domain.StarGiftCollectibleAttribute{{
Kind: domain.StarGiftCollectibleModel, Name: "Base", RarityKind: domain.StarGiftRarityPermille, RarityPermille: 1000,
Document: collectibleTestDocumentPtr(baseDocumentID+1, "model.tgs"), Blob: collectibleTestBlobPtr(baseDocumentID+1, "model"), Animation: collectibleTestAnimationPtr("model.tgs"),
}},
Patterns: []domain.StarGiftCollectibleAttribute{{
Kind: domain.StarGiftCollectiblePattern, Name: "Orbit", RarityKind: domain.StarGiftRarityPermille, RarityPermille: 1000,
Document: collectibleTestPatternDocumentPtr(baseDocumentID+2, "pattern.tgs"), Blob: collectibleTestBlobPtr(baseDocumentID+2, "pattern"), Animation: collectibleTestAnimationPtr("pattern.tgs"),
}},
Backdrops: []domain.StarGiftCollectibleAttribute{{
Kind: domain.StarGiftCollectibleBackdrop, Name: "Night", BackdropID: 91, RarityKind: domain.StarGiftRarityPermille, RarityPermille: 1000,
CenterColor: 0x112233, EdgeColor: 0x223344, PatternColor: 0x334455, TextColor: 0xffffff,
}, {
Kind: domain.StarGiftCollectibleBackdrop, Name: "Day", BackdropID: 92, RarityKind: domain.StarGiftRarityPermille, RarityPermille: 500,
CenterColor: 0xaabbcc, EdgeColor: 0x778899, PatternColor: 0xddeeff, TextColor: 0x111111,
}},
Actor: "integration", CommandID: "delete-upgrade-pool-" + suffix,
}); err != nil {
t.Fatalf("publish delete pool: %v", err)
}
messages := NewMessageStore(pool)
saved := createCollectibleSavedGift(t, ctx, messages, gifts, entry.Gift, domain.SavedStarGift{
Owner: ownerPeer, FromUserID: sender.ID, GiftID: entry.Gift.ID, RevisionID: entry.Gift.RevisionID,
Date: now, ConvertStars: 0, Message: "deleted gift",
})
lifecycle := NewStarGiftLifecycleStore(pool, messages, 1_000_000)
ref := domain.SavedStarGiftRef{Owner: ownerPeer, MsgID: saved.MsgID}
if _, err := lifecycle.DeleteStarGift(ctx, domain.StarGiftDeleteRequest{UserID: owner.ID, Ref: ref, Date: now}); err != nil {
t.Fatalf("delete plain gift: %v", err)
}
stars := NewStarsStore(pool)
if _, _, err := stars.EnsureGrant(ctx, owner.ID, 1000, now); err != nil {
t.Fatalf("grant upgrade stars: %v", err)
}
before, err := stars.GetBalance(ctx, owner.ID)
if err != nil {
t.Fatalf("read balance: %v", err)
}
upgrades := NewStarGiftUpgradeStore(pool, messages)
if _, err := upgrades.UpgradeStarGift(ctx, domain.StarGiftUpgradeRequest{
UserID: owner.ID, Ref: ref, ChargeStars: 100, FormID: 8802,
CommandKey: "delete-then-upgrade-" + suffix, Date: now + 1,
}); !errors.Is(err, domain.ErrStarGiftNotFound) {
t.Fatalf("upgrade deleted gift err = %v, want ErrStarGiftNotFound", err)
}
after, err := stars.GetBalance(ctx, owner.ID)
if err != nil {
t.Fatalf("read balance: %v", err)
}
if after.Balance != before.Balance {
t.Fatalf("balance %d -> %d, want unchanged", before.Balance, after.Balance)
}
var status string
var uniqueGiftID *int64
if err := pool.QueryRow(ctx, `SELECT lifecycle_status, unique_gift_id FROM peer_star_gifts WHERE id = $1`, saved.ID).Scan(&status, &uniqueGiftID); err != nil {
t.Fatalf("read deleted gift: %v", err)
}
if status != string(domain.StarGiftLifecycleDeleted) || uniqueGiftID != nil {
t.Fatalf("deleted gift = %q unique=%v, want deleted without a collectible", status, uniqueGiftID)
}
var minted, commands int
if err := pool.QueryRow(ctx, `SELECT count(*) FROM unique_star_gifts WHERE gift_id = $1`, entry.Gift.ID).Scan(&minted); err != nil {
t.Fatalf("count collectibles: %v", err)
}
if err := pool.QueryRow(ctx, `SELECT count(*) FROM star_gift_upgrade_commands WHERE source_saved_gift_id = $1`, saved.ID).Scan(&commands); err != nil {
t.Fatalf("count upgrade commands: %v", err)
}
if minted != 0 || commands != 0 {
t.Fatalf("collectibles=%d upgrade commands=%d after a refused upgrade, want 0/0", minted, commands)
}
}
// TestStarGiftDeleteRefreshesCollectionHashPostgres pins the collection side of
// payments.deleteStarGift: removing the membership must refresh the stored
// collection hash under the owner advisory lock, otherwise
@@ -405,6 +405,11 @@ func (s *StarGiftUpgradeStore) UpgradeStarGift(ctx context.Context, req domain.S
if locked.UniqueGiftID != 0 {
return domain.ErrStarGiftAlreadyUpgraded
}
// A deleted gift keeps its row for the audit trail but has left the
// profile; it is no longer something the owner can upgrade.
if !locked.LifecycleStatus.Live() {
return domain.ErrStarGiftNotFound
}
revision, err := lockActiveCollectibleRevision(ctx, tx, locked.GiftID)
if err != nil {