SEP-10: Accept challenge transactions through clock drift (#667)

Accept challenge transactions that are 5 minutes off from their min and max time, to account for client clocks not matching server clocks.

CHANGELOG.md

@@ -4,6 +4,11 @@ A breaking change will get clearly marked in this log.
 
 ## Unreleased
 
+## [v8.2.4](https://github.com/stellar/js-stellar-sdk/compare/v8.2.3...v8.2.4)
+
+### Fix
+- Utils.readTransactionTx now checks timebounds with a 5-minute grace period to account for clock drift.
+
 ## [v8.2.3](https://github.com/stellar/js-stellar-sdk/compare/v8.2.2...v8.2.3)
 
 ### Fix

package.json

@@ -1,6 +1,6 @@
 {
   "name": "stellar-sdk",
-  "version": "8.2.3",
+  "version": "8.2.4",
   "description": "stellar-sdk is a library for working with the Stellar Horizon server.",
   "main": "./lib/index.js",
   "types": "./lib/index.d.ts",

src/utils.ts

@@ -187,7 +187,8 @@ export namespace Utils {
       );
     }
 
-    if (!validateTimebounds(transaction)) {
+    // give a small grace period for the transaction time to account for clock drift
+    if (!validateTimebounds(transaction, 60 * 5)) {
       throw new InvalidSep10ChallengeError("The transaction has expired");
     }
 
@@ -631,7 +632,10 @@ export namespace Utils {
    * @param {Transaction} transaction the transaction whose timebonds will be validated.
    * @returns {boolean} returns true if the current time is within the transaction's [minTime, maxTime] range.
    */
-  function validateTimebounds(transaction: Transaction): boolean {
+  function validateTimebounds(
+    transaction: Transaction,
+    gracePeriod: number = 0,
+  ): boolean {
     if (!transaction.timeBounds) {
       return false;
     }
@@ -640,7 +644,8 @@ export namespace Utils {
     const { minTime, maxTime } = transaction.timeBounds;
 
     return (
-      now >= Number.parseInt(minTime, 10) && now <= Number.parseInt(maxTime, 10)
+      now >= Number.parseInt(minTime, 10) - gracePeriod &&
+      now <= Number.parseInt(maxTime, 10) + gracePeriod
     );
   }
 }

test/unit/utils_test.js

@@ -556,7 +556,8 @@ describe('Utils', function() {
         "testanchor.stellar.org"
       );
 
-      clock.tick(350000);
+      // Note that this is greater than the grace period of 5 minutes (600 seconds)
+      clock.tick(1000 * 1000);
 
       const transaction = new StellarSdk.Transaction(
         challenge,
@@ -583,6 +584,28 @@ describe('Utils', function() {
       );
     });
 
+
+    it("does NOT throw errors when the user is slightly out of minTime", function() {
+      clock.tick(1626888681 * 1000);
+
+      // this challenge from Stablex's testnet env, collected 2021-07-21T17:31:21.530Z,
+      // is erroring, and we want to know if it's a bug on our side or in the sdk
+      const signedChallenge = "AAAAAgAAAADZJunw2QO9LzjqagEjh/mpWG8Us5nOb+gc6wOex8G+IwAAAGQAAAAAAAAAAAAAAAEAAAAAYPhZ6gAAAXrKHz2UAAAAAAAAAAEAAAABAAAAAJyknd/qYHdzX6iV3TkHlh/usJUr5/U8cRsfVNqaruBAAAAACgAAAB50ZXN0bmV0LXNlcC5zdGFibGV4LmNsb3VkIGF1dGgAAAAAAAEAAABAaEs3QUZieUFCZzBEekx0WnpTVXJkcEhWOXdkdExXUkwxUHFFOW5QRVIrZVlaZzQvdDJlc3drclpBc0ZnTnp5UQAAAAAAAAABx8G+IwAAAEA8I5qQ+/HHXoHrULlg1ODTiCEQ92GQrVBFaB40OKxJhTf1c597AuKLHhJ3c4TNdSp1rjLGbk7qUuhjauxUuH0N";
+
+      expect(() =>
+        StellarSdk.Utils.readChallengeTx(
+          signedChallenge,
+          "GDMSN2PQ3EB32LZY5JVACI4H7GUVQ3YUWOM4437IDTVQHHWHYG7CGA5Z",
+          StellarSdk.Networks.TESTNET,
+          "testnet-sep.stablex.cloud",
+          "staging-transfer-server.zetl.network"
+        ),
+      ).not.to.throw(
+        StellarSdk.InvalidSep10ChallengeError,
+        /The transaction has expired/,
+      );
+    });
+
     it("home domain string matches transaction\'s operation key name", function() {
       let serverKP = StellarSdk.Keypair.random();
       let clientKP = StellarSdk.Keypair.random();