-
Notifications
You must be signed in to change notification settings - Fork 3
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix lack of idempotency on bump_iap_set_expiry
This commit fixes an important bug in the logic of bump_iap_set_expiry. I discovered that it lacks idempotency when sending the same IAP (In-app purchase) receipt more than once. It would cause the end user to end up with a higher expiry date than entitled to them. Since there was no way to create a logic that solves this by only using a simple expiry date number, the system was changed to keep track of transactions, and then calculate the expiry date on the fly. Furthermore, these changes improve the overall system as mistakes such as this one become more easily fixable, as the transaction data is a much better source of truth from which we can calculate expiry. Multiple changes were made to support this logical and structural change: 1. A new set of functions were introduced to perform the calculation in O(N) time 2. New tests and test cases were introduced to cover both logic and the original bug 3. bump_expiry and bump_iap_set_expiry were replaced by a single function that appends a transaction to the user's transaction list 4. A new standard transaction data type was introduced 5. Low-level functions that fetch and write user data on the database were changed to calculate or handle expiry dates on the fly to reduce other API changes to a minimum 6. IAP and LN successful purchase logics were modified to generate transaction data for the transaction history. 7. Logic was added to handle the old legacy expiry date data as a "legacy" transaction. Testing 1 ---------- PASS Made sure all automated tests are passing (They are quite extensive and realistic at this point) Testing 2 ---------- PASS Devices: iPhone 13 Mini (physical device), iPhone 15 simulator iOS: 17.3.1 and 17.2 damus-api: This commit Damus: Tip of master (5e530bfc9c584dcb93729b6a21863c97125ffb21, very close to 1.8(1)) Damus-website: 927bf8bacae71d59ab0c5f2386e1c50b8e172790 (Equivalent to the one released with 1.7.2) Setup: Local server setup, App Store sandbox environment. Coverage: 1. Loaded a copy of the production DB on the local test environment, and verified: 1. A lot of the users I knew had Damus Purple membership still had their star next to them after loading the new version. 2. My own account was displaying correct information 3. I still saw reminder notifications to renew as expected 4. Going through LN checkout worked and lead to the correct expiry date to be shown 5. Going through the LN checkout a second time worked and lead to the expected extension of expiry 6. Saw no crashes on the server 7. Saw no lags on the server 2. Loaded an older test database I used a while ago with an older version 1. My own account was displaying correct information 2. Tried renewing with an IAP purchase via Sandbox. It worked as expected 3. Expiry date is as expected Changelog-Fixed: Fixed IAP receipt verification idempotency problems Changelog-Changed: Changed structure of expiry to be a function of transaction data Signed-off-by: Daniel D’Aquino <[email protected]> Signed-off-by: William Casarin <[email protected]>
- Loading branch information
1 parent
8e7f076
commit 346acfb
Showing
9 changed files
with
736 additions
and
92 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| "use strict"; | ||
| // @ts-check | ||
|
|
||
|
|
||
| /** @typedef {Object} Transaction | ||
| * @property {"iap" | "ln" | "legacy"} type - The type of transaction | ||
| * @property {string} id - The id of the transaction (Apple IAP transaction ID for iap, Checkout ID for ln, `0` for legacy) | ||
| * @property {number | null} start_date - The start date of the transaction if transaction has a fixed start and end (IAP and legacy), null otherwise (Unix timestamp in seconds) | ||
| * @property {number | null} end_date - The end date of the transaction if transaction has a fixed start and end (IAP and legacy), null otherwise (Unix timestamp in seconds) | ||
| * @property {number | null} purchased_date - The date of the transaction (Applies to LN and IAP only, Unix timestamp in seconds) | ||
| * @property {number | null} duration - The duration of the transaction (Applies to LN only, measured in seconds) | ||
| */ | ||
|
|
||
|
|
||
| /** Calculates the expiry date given a transaction history | ||
| * @param {Transaction[]} transaction_history - The transaction history | ||
| * @returns {number | null} - The expiry date of the transaction history (Unix timestamp in seconds), null if no expiry date can be calculated | ||
| */ | ||
| function calculate_expiry_date_from_history(transaction_history) { | ||
| // make a deep copy of the transaction history so that we don't modify the original objects | ||
| const transaction_history_copy = deep_copy_unique_transaction_history(transaction_history); | ||
|
|
||
| // sort the transaction history by earliest date | ||
| var remaining_transactions = transaction_history_copy.sort((a, b) => get_earliest_date_from_transaction(a) - get_earliest_date_from_transaction(b)); | ||
| var time_cursor = null; | ||
| var flexible_credits_remaining = 0; | ||
|
|
||
| for (var i = 0; i < remaining_transactions.length; i++) { | ||
| var transaction = remaining_transactions[i]; | ||
|
|
||
| // Move time cursor and count flexible credits available. | ||
| if (is_transaction_fixed_schedule(transaction)) { | ||
| time_cursor = Math.max(time_cursor, transaction.end_date); | ||
| } | ||
| else if (is_transaction_flexible_schedule(transaction)) { | ||
| flexible_credits_remaining += transaction.duration; | ||
| time_cursor = Math.max(time_cursor, transaction.purchased_date); | ||
| } | ||
|
|
||
| // Check if there is a gap between the time cursor and the next transaction, then consume flexible credits. | ||
| if (i < remaining_transactions.length - 1) { | ||
| var next_transaction = remaining_transactions[i + 1]; | ||
| let earliest_date_from_next_transaction = get_earliest_date_from_transaction(next_transaction); | ||
| if (time_cursor < earliest_date_from_next_transaction && flexible_credits_remaining > 0) { | ||
| flexible_credits_remaining -= Math.min(earliest_date_from_next_transaction - time_cursor, flexible_credits_remaining); | ||
| time_cursor = earliest_date_from_next_transaction; | ||
| } | ||
| } | ||
| else { | ||
| // This is the last transaction. Consume all remaining flexible credits. | ||
| time_cursor += flexible_credits_remaining; | ||
| } | ||
| } | ||
|
|
||
| return time_cursor; | ||
| } | ||
|
|
||
|
|
||
| /** Checks if a transaction is fixed schedule | ||
| * @param {Transaction} transaction - The transaction to be checked | ||
| */ | ||
| function is_transaction_fixed_schedule(transaction) { | ||
| return transaction.start_date !== null && transaction.start_date !== undefined && transaction.end_date !== null && transaction.end_date !== undefined; | ||
| } | ||
|
|
||
|
|
||
| /** Checks if a transaction is flexible schedule | ||
| * @param {Transaction} transaction - The transaction to be checked | ||
| */ | ||
| function is_transaction_flexible_schedule(transaction) { | ||
| return transaction.purchased_date !== null && transaction.purchased_date !== undefined && transaction.duration !== null && transaction.duration !== undefined && !is_transaction_fixed_schedule(transaction); | ||
| } | ||
|
|
||
|
|
||
| /** Returns the earliest date from a transaction | ||
| * @param {Transaction} transaction - The transaction | ||
| * @returns {number | null} - The earliest date from the transaction (Unix timestamp in seconds), null if the transaction is null | ||
| */ | ||
| function get_earliest_date_from_transaction(transaction) { | ||
| if (is_transaction_fixed_schedule(transaction)) { | ||
| return transaction.start_date; | ||
| } | ||
| else if (is_transaction_flexible_schedule(transaction)) { | ||
| return transaction.purchased_date; | ||
| } | ||
| else { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| /** Deep copies a transaction history. More efficient than a JSON parsing roundtrip | ||
| * @param {Transaction[]} transaction_history - The transaction history | ||
| * @returns {Transaction[]} - The deep copied transaction history | ||
| */ | ||
| function deep_copy_unique_transaction_history(transaction_history) { | ||
| // @type {Transaction[]} | ||
| var new_transaction_history = []; | ||
| // @type {Set<string>} | ||
| var unique_transaction_ids = new Set(); | ||
| for (var i = 0; i < transaction_history.length; i++) { | ||
| const unique_id = transaction_history[i].type + "_" + transaction_history[i].id; | ||
| if (unique_transaction_ids.has(unique_id)) { | ||
| continue; | ||
| } | ||
| unique_transaction_ids.add(unique_id); | ||
| new_transaction_history.push({ | ||
| type: transaction_history[i].type, | ||
| id: transaction_history[i].id, | ||
| start_date: transaction_history[i].start_date, | ||
| end_date: transaction_history[i].end_date, | ||
| purchased_date: transaction_history[i].purchased_date, | ||
| duration: transaction_history[i].duration | ||
| }); | ||
| } | ||
|
|
||
| return new_transaction_history; | ||
| } | ||
|
|
||
|
|
||
| module.exports = { | ||
| calculate_expiry_date_from_history, deep_copy_unique_transaction_history | ||
| } |
Oops, something went wrong.