fix: let internal govobj/vote inv request trackers expire after 60 sec

[UdjinM6]

Issue being fixed or feature implemented

Another issue noticed during recent sb voting period. Lots of inv are coming from peers that never send the object/vote they announced. Let's not wait for these forever, 60 seconds should be enough.

What was done?

Add a "timer" to each request.

How Has This Been Tested?

Run tests, run a MN on mainnet and check logs.

Breaking Changes

n/a

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[knst]

utACK 75f4115

[knst]

if set is giving information what that's just a set of requested object, the map as a member name prefix doesn't give a context.

Consider renaming mapRequestedObjects to requestedObjectsWithTime or timeOfRequestedObjects or something else.

Same for AcceptMessage: mapHash -> hashesWithTime ?

[UdjinM6]

Good point 👍 Also the logic for these 2 maps is essentially the same so I took it a bit further and squashed them into one, see dea1395.

[knst]

note for myself: GetTime return mocked time here

[knst]

utACK dea1395

[PastaPastaPasta]

maybe we keep the times here in std::chrono types?

[coolaj86]

Could someone please explain simply what this fix means in terms of the user pipeline for gobject prepare; gobject submit?
(which I hope is no change)

Real-World, Implemented-in-Production Use Case

I created the following proposal with a time backdated to match start_epoch (or otherwise time is one day in the past on the same hour as start_epoch):

• https://rpc.digitalcash.dev/#?method=gobject&params=["get","831672df4230c3578c10ab4c939d912596ca8b9328a93912ddf596699d6a3af9"]&submit

This was to avoid the following, documented-as-dangerous UX bug:

• https://github.com/dashpay/dash/blob/v21.1.x/src/rpc/governance.cpp#L168-L171

    // This command is dangerous because it consumes 5 DASH irreversibly.
    // If params are lost, it's very hard to bruteforce them and yet
    // users ignore all instructions on dashcentral etc. and do not save them...
    // Let's log them here and hope users do not mess with debug.log

The code for generating the timestamps to circumvent that bug is here:

• https://github.com/dashhive/DashGov.js/blob/35fc77ecef083776e81367330c57a98760f1f6cd/dashgov.js#L395-L420

/**
   * Creates a draft object with reasonable and default values
   * @param {Uint53} now - use Date.now(), except in testing
   * @param {Uint53} startEpochMs - used to create a deterministic gobject time
   * @param {GObjectData} data - will be sorted and hex-ified
   * @param {GObject} [gobj] - override values
   */
  DashGov.proposal.draft = function (now, startEpochMs, data, gobj) {
    let dataHex = gobj?.dataHex || DashGov.proposal.sortAndEncodeJson(data);
    let time = DashGov.proposal._selectKnownTime(now, startEpochMs);

    /** @type {DashGov.GObject} */
    let normalGObj = {
      hashParent: 0,
      revision: 1,
      time: time,
      dataHex: "",
      masternodeOutpoint: null,
      collateralTxId: null,
      collateralTxOutputIndex: null,
      signature: null,
    };
    Object.assign(normalGObj, gobj, { dataHex });

    return normalGObj;
  };

/**
   * The arbitrary use of random times is a leading cause of lost money during
   * the proposal process, so instead we use the 'start epoch' when appropriate,
   * or otherwise a UTC day interval of the start epoch
   * @param {Uint53} now
   * @param {Uint53} startMs
   */
  DashGov.proposal._selectKnownTime = function (now, startMs) {
    let startEpochDate = new Date(startMs);
    let today = new Date();
    if (today < startEpochDate) {
      let date = today.getUTCDate();
      startEpochDate.setUTCFullYear(today.getUTCFullYear());
      startEpochDate.setUTCMonth(today.getUTCMonth());
      startEpochDate.setUTCDate(date - 1);
    }
    let knownTimeMs = startEpochDate.valueOf();
    let knownSecs = knownTimeMs / 1000;
    knownSecs = Math.floor(knownSecs);

    return knownSecs;
  };

Assumption

I believe what this issue is saying is just that it's going to ignore inventory spam based on actual MN message time from the (supposed) gobject submit that should be sending inventory via p2p and gobject via the MN network simultaneously. So I believe it won't affect this sort of fix, but I'd like an ACK... or a change to the code to not cement the UX bug further in place at the protocol layer.

edit: hmm... not sure why github isn't unfurling those code blocks like it normally does, updated with code

[UdjinM6]

@coolaj86 this PR has nothing to do with the proposal data itself, it's dealing with an observed misbehaviour on p2p level only. Basically, a peer says "I have a governance object/vote with hash X", we remember this and request it. The object with hash X never arrives but we kept waiting for it forever (not really waiting in networking terms, just remembering that we asked for it earlier). This PR simply lets us forget about such requests after 60 seconds.

[PastaPastaPasta]

let's rebase I guess to have CI green? may just be a flaky failure, restarting.

[PastaPastaPasta]

utACK d41d87a; changes to tests for this expiration logic would be appreciated

[knst]

utACK 1f4e1a1