Merge pull request #1003 from golemfactory/bugfix/JST-1011/demand-ref…

…reshing-with-wrong-expiration

fix(market): fixed demand refresh logic that produced unusable counte…

examples/advanced/manual-pools.ts

@@ -1,7 +1,8 @@
 import { Allocation, DraftOfferProposalPool, GolemNetwork } from "@golem-sdk/golem-js";
 import { pinoPrettyLogger } from "@golem-sdk/pino-logger";
 
-const RENT_HOURS = 0.25;
+const RENTAL_DURATION_HOURS = 0.25;
+const ALLOCATION_DURATION_HOURS = RENTAL_DURATION_HOURS + 0.25;
 
 const demandOptions = {
   demand: {
@@ -13,7 +14,7 @@ const demandOptions = {
     },
   },
   market: {
-    rentHours: RENT_HOURS,
+    rentHours: RENTAL_DURATION_HOURS,
     pricing: {
       model: "linear",
       maxStartPrice: 1,
@@ -31,15 +32,25 @@ const demandOptions = {
   const glm = new GolemNetwork({
     logger,
   });
+
+  console.assert(
+    ALLOCATION_DURATION_HOURS > RENTAL_DURATION_HOURS,
+    "Always create allocations that will live longer than the planned rental duration",
+  );
+
   let allocation: Allocation | undefined;
 
   try {
     await glm.connect();
 
-    allocation = await glm.payment.createAllocation({ budget: 1, expirationSec: RENT_HOURS * 60 * 60 });
+    allocation = await glm.payment.createAllocation({ budget: 1, expirationSec: ALLOCATION_DURATION_HOURS * 60 * 60 });
 
     const proposalPool = new DraftOfferProposalPool({ minCount: 1 });
-    const demandSpecification = await glm.market.buildDemandDetails(demandOptions.demand, allocation);
+    const demandSpecification = await glm.market.buildDemandDetails(
+      demandOptions.demand,
+      demandOptions.market,
+      allocation,
+    );
 
     const draftProposal$ = glm.market.collectDraftOfferProposals({
       demandSpecification,

examples/advanced/reuse-allocation.ts

@@ -4,7 +4,16 @@
  */
 import { MarketOrderSpec, GolemNetwork } from "@golem-sdk/golem-js";
 import { pinoPrettyLogger } from "@golem-sdk/pino-logger";
+
 (async () => {
+  const ALLOCATION_DURATION_HOURS = 1;
+  const RENTAL_DURATION_HOURS = 0.5;
+
+  console.assert(
+    ALLOCATION_DURATION_HOURS > RENTAL_DURATION_HOURS,
+    "Always create allocations that will live longer than the planned rental duration",
+  );
+
   const glm = new GolemNetwork({
     logger: pinoPrettyLogger({
       level: "info",
@@ -16,15 +25,15 @@ import { pinoPrettyLogger } from "@golem-sdk/pino-logger";
 
     const allocation = await glm.payment.createAllocation({
       budget: 1,
-      expirationSec: 3600,
+      expirationSec: ALLOCATION_DURATION_HOURS * 60 * 60,
     });
 
     const firstOrder: MarketOrderSpec = {
       demand: {
         workload: { imageTag: "golem/alpine:latest" },
       },
       market: {
-        rentHours: 0.5,
+        rentHours: RENTAL_DURATION_HOURS,
         pricing: {
           model: "burn-rate",
           avgGlmPerHour: 0.5,
@@ -40,7 +49,7 @@ import { pinoPrettyLogger } from "@golem-sdk/pino-logger";
         workload: { imageTag: "golem/alpine:latest" },
       },
       market: {
-        rentHours: 0.5,
+        rentHours: RENTAL_DURATION_HOURS,
         pricing: {
           model: "burn-rate",
           avgGlmPerHour: 0.5,

examples/advanced/step-by-step.ts

@@ -24,23 +24,31 @@ import { filter, map, switchMap, take } from "rxjs";
     logger,
   });
 
+  const RENTAL_DURATION_HOURS = 5 / 60;
+  const ALLOCATION_DURATION_HOURS = RENTAL_DURATION_HOURS + 0.25;
+
+  console.assert(
+    ALLOCATION_DURATION_HOURS > RENTAL_DURATION_HOURS,
+    "Always create allocations that will live longer than the planned rental duration",
+  );
+
   let allocation: Allocation | undefined;
   try {
     await glm.connect();
 
     // Define the order that we're going to place on the market
+
     const order: MarketOrderSpec = {
       demand: {
         workload: {
           imageTag: "golem/alpine:latest",
           minCpuCores: 1,
           minMemGib: 2,
         },
-        expirationSec: 30 * 60,
       },
       market: {
         // We're only going to rent the provider for 5 minutes max
-        rentHours: 5 / 60,
+        rentHours: RENTAL_DURATION_HOURS,
         pricing: {
           model: "linear",
           maxStartPrice: 1,
@@ -51,13 +59,14 @@ import { filter, map, switchMap, take } from "rxjs";
     };
     // Allocate funds to cover the order, we will only pay for the actual usage
     // so any unused funds will be returned to us at the end
+
     allocation = await glm.payment.createAllocation({
       budget: glm.market.estimateBudget({ order, maxAgreements: 1 }),
-      expirationSec: order.market.rentHours * 60 * 60,
+      expirationSec: ALLOCATION_DURATION_HOURS * 60 * 60,
     });
 
     // Convert the human-readable order to a protocol-level format that we will publish on the network
-    const demandSpecification = await glm.market.buildDemandDetails(order.demand, allocation);
+    const demandSpecification = await glm.market.buildDemandDetails(order.demand, order.market, allocation);
 
     // Publish the order on the market
     // This methods creates and observable that publishes the order and refreshes it every 30 minutes.

src/experimental/deployment/deployment.ts

@@ -153,7 +153,12 @@ export class Deployment {
         ? this.networks.get(pool.options?.deployment.network)
         : undefined;
 
-      const demandSpecification = await this.modules.market.buildDemandDetails(pool.options.demand, allocation);
+      const demandSpecification = await this.modules.market.buildDemandDetails(
+        pool.options.demand,
+        pool.options.market,
+        allocation,
+      );
+
       const proposalPool = new DraftOfferProposalPool({
         logger: this.logger,
         validateOfferProposal: pool.options.market.offerProposalFilter,

src/golem-network/golem-network.ts

@@ -5,8 +5,9 @@ import {
   IMarketApi,
   MarketModule,
   MarketModuleImpl,
-  MarketOptions,
+  MarketModuleOptions,
   OfferProposal,
+  OrderMarketOptions,
 } from "../market";
 import { Allocation, IPaymentApi, PaymentModule, PaymentModuleImpl, PaymentModuleOptions } from "../payment";
 import { ActivityModule, ActivityModuleImpl, ExeUnitOptions, IActivityApi, IFileServer } from "../activity";
@@ -27,7 +28,7 @@ import { AgreementRepository } from "../shared/yagna/repository/agreement-reposi
 import { ProposalRepository } from "../shared/yagna/repository/proposal-repository";
 import { CacheService } from "../shared/cache/CacheService";
 import { DemandRepository } from "../shared/yagna/repository/demand-repository";
-import { BuildDemandOptions, IDemandRepository } from "../market/demand/demand";
+import { IDemandRepository, OrderDemandOptions } from "../market/demand";
 import { GftpServerAdapter } from "../shared/storage/GftpServerAdapter";
 import {
   GftpStorageProvider,
@@ -77,24 +78,37 @@ export interface GolemNetworkOptions {
    * `DEBUG` environment variable to `golem-js:*`.
    */
   logger?: Logger;
+
   /**
    * Set the API key and URL for the Yagna API.
    */
   api?: {
     key?: string;
     url?: string;
   };
+
   /**
    * Set payment-related options.
+   *
    * This is where you can specify the network, payment driver and more.
    * By default, the network is set to the `holesky` test network.
    */
   payment?: Partial<PaymentModuleOptions>;
+
+  /**
+   * Set market related options.
+   *
+   * This is where you can globally specify several options that determine how the SDK will
+   * interact with the market.
+   */
+  market?: Partial<MarketModuleOptions>;
+
   /**
    * Set the data transfer protocol to use for file transfers.
    * Default is `gftp`.
    */
   dataTransferProtocol?: DataTransferProtocol;
+
   /**
    * Override some of the services used by the GolemNetwork instance.
    * This is useful for testing or when you want to provide your own implementation of some services.
@@ -124,10 +138,11 @@ type AllocationOptions = {
  * Represents the order specifications which will result in access to ResourceRental.
  */
 export interface MarketOrderSpec {
-  demand: BuildDemandOptions;
-  market: MarketOptions;
+  demand: OrderDemandOptions;
+  market: OrderMarketOptions;
   activity?: ResourceRentalOptions["activity"];
   payment?: ResourceRentalOptions["payment"] & AllocationOptions;
+  /** The network that should be used for communication between the resources rented as part of this order */
   network?: Network;
 }
 
@@ -273,13 +288,13 @@ export class GolemNetwork {
         fileServer: this.options.override?.fileServer || new GftpServerAdapter(this.storageProvider),
       };
       this.network = getFactory(NetworkModuleImpl, this.options.override?.network)(this.services);
-      this.market = getFactory(
-        MarketModuleImpl,
-        this.options.override?.market,
-      )({
-        ...this.services,
-        networkModule: this.network,
-      });
+      this.market = getFactory(MarketModuleImpl, this.options.override?.market)(
+        {
+          ...this.services,
+          networkModule: this.network,
+        },
+        this.options.market,
+      );
       this.payment = getFactory(PaymentModuleImpl, this.options.override?.payment)(this.services, this.options.payment);
       this.activity = getFactory(ActivityModuleImpl, this.options.override?.activity)(this.services);
       this.rental = getFactory(
@@ -362,14 +377,27 @@ export class GolemNetwork {
   }): Promise<Allocation> {
     if (!order.payment?.allocation) {
       const budget = this.market.estimateBudget({ order, maxAgreements });
+
+      /**
+       * We need to create allocations that will exist longer than the agreements made.
+       *
+       * Without this in the event of agreement termination due to its expiry,
+       * the invoice for the agreement arrives, and we try to accept the invoice with
+       * an allocation that already expired (had the same expiration time as the agreement),
+       * which leads to unpaid invoices.
+       */
+      const EXPIRATION_BUFFER_MINUTES = 15;
+
       return this.payment.createAllocation({
         budget,
-        expirationSec: order.market.rentHours * 60 * 60,
+        expirationSec: order.market.rentHours * (60 + EXPIRATION_BUFFER_MINUTES) * 60,
       });
     }
+
     if (typeof order.payment.allocation === "string") {
       return this.payment.getAllocation(order.payment.allocation);
     }
+
     return order.payment.allocation;
   }
 
@@ -434,7 +462,7 @@ export class GolemNetwork {
       allocation = await this.getAllocationFromOrder({ order, maxAgreements: 1 });
       signal.throwIfAborted();
 
-      const demandSpecification = await this.market.buildDemandDetails(order.demand, allocation);
+      const demandSpecification = await this.market.buildDemandDetails(order.demand, order.market, allocation);
       const draftProposal$ = this.market.collectDraftOfferProposals({
         demandSpecification,
         pricing: order.market.pricing,
@@ -554,7 +582,7 @@ export class GolemNetwork {
       allocation = await this.getAllocationFromOrder({ order, maxAgreements });
       signal.throwIfAborted();
 
-      const demandSpecification = await this.market.buildDemandDetails(order.demand, allocation);
+      const demandSpecification = await this.market.buildDemandDetails(order.demand, order.market, allocation);
 
       const draftProposal$ = this.market.collectDraftOfferProposals({
         demandSpecification,
@@ -563,6 +591,8 @@ export class GolemNetwork {
       });
       subscription = proposalPool.readFrom(draftProposal$);
 
+      const rentSeconds = order.market.rentHours * 60 * 60;
+
       resourceRentalPool = this.rental.createResourceRentalPool(proposalPool, allocation, {
         poolSize,
         network: order.network,
@@ -572,9 +602,10 @@ export class GolemNetwork {
           exeUnit: { setup, teardown },
         },
         agreementOptions: {
-          expirationSec: order.market.rentHours * 60 * 60,
+          expirationSec: rentSeconds,
         },
       });
+
       this.cleanupTasks.push(cleanup);
 
       return resourceRentalPool;

src/market/agreement/agreement.test.ts

@@ -33,7 +33,6 @@ const demand = new Demand(
       properties: [],
     },
     "erc20-holesky-tglm",
-    30 * 60,
   ),
 );
 

src/market/demand/demand.ts

@@ -82,7 +82,7 @@ export interface BasicDemandPropertyConfig {
   midAgreementPaymentTimeoutSec: number;
 }
 
-export type BuildDemandOptions = Partial<{
+export type OrderDemandOptions = Partial<{
   /** Demand properties related to the activities that will be executed on providers */
   workload: Partial<WorkloadDemandDirectorConfigOptions>;
   /** Demand properties that determine payment related terms & conditions of the agreement */
@@ -104,7 +104,6 @@ export class DemandSpecification {
     /** Represents the low level demand request body that will be used to subscribe for offers matching our "computational resource needs" */
     public readonly prototype: DemandBodyPrototype,
     public readonly paymentPlatform: string,
-    public readonly expirationSec: number,
   ) {}
 }
 

src/market/demand/directors/basic-demand-director-config.test.ts

@@ -1,12 +1,11 @@
 import { BasicDemandDirectorConfig } from "./basic-demand-director-config";
 
 describe("BasicDemandDirectorConfig", () => {
-  test("should throw user error if expiration option is invalid", () => {
-    expect(() => {
-      new BasicDemandDirectorConfig({
-        expirationSec: -3,
-        subnetTag: "public",
-      });
-    }).toThrow("The demand expiration time has to be a positive integer");
+  test("it sets the subnet tag property", () => {
+    const config = new BasicDemandDirectorConfig({
+      subnetTag: "public",
+    });
+
+    expect(config.subnetTag).toBe("public");
   });
 });

src/market/demand/directors/basic-demand-director-config.ts

@@ -1,28 +1,19 @@
 import { BaseConfig } from "./base-config";
-import { GolemConfigError } from "../../../shared/error/golem-error";
 import * as EnvUtils from "../../../shared/utils/env";
 
 export interface BasicDemandDirectorConfigOptions {
-  expirationSec: number;
+  /** Determines which subnet tag should be used for the offer/demand matching */
   subnetTag: string;
 }
 
 export class BasicDemandDirectorConfig extends BaseConfig implements BasicDemandDirectorConfigOptions {
-  public readonly expirationSec = 30 * 60; // 30 minutes
   public readonly subnetTag: string = EnvUtils.getYagnaSubnet();
 
   constructor(options?: Partial<BasicDemandDirectorConfigOptions>) {
     super();
 
-    if (options?.expirationSec) {
-      this.expirationSec = options.expirationSec;
-    }
     if (options?.subnetTag) {
       this.subnetTag = options.subnetTag;
     }
-
-    if (!this.isPositiveInt(this.expirationSec)) {
-      throw new GolemConfigError("The demand expiration time has to be a positive integer");
-    }
   }
 }

src/market/demand/directors/basic-demand-director.ts

@@ -8,7 +8,6 @@ export class BasicDemandDirector implements IDemandDirector {
   apply(builder: DemandBodyBuilder) {
     builder
       .addProperty("golem.srv.caps.multi-activity", true)
-      .addProperty("golem.srv.comp.expiration", Date.now() + this.config.expirationSec * 1000)
       .addProperty("golem.node.debug.subnet", this.config.subnetTag);
 
     builder