warp-contracts / warp

An implementation of the Arweave SmartWeave smart contracts protocol.
MIT License
158 stars 44 forks source link

[BUG] Different state result from readState when using JS and Node #456

Closed arcj0 closed 10 months ago

arcj0 commented 1 year ago

Describe the bug Seeing an older state when reading contract from node.

To Reproduce I've seen this with several simple contracts (no internal writes). Here's one on testnet: "t5VFXv-YC4c4U5JhAk6kChQE8XArkj44DIbOh1kCvj8"

Expected behavior JS Validaties: "validity": { "IC46XQwIu19jFhsKCsE_q5XUNAnUykCYpbp64Hp6Ueg": false, "rY5HBPNHn2qNEEjT7JTxgCI3RBqEKcicfhXSdzBgs3I": false, "Hg6oXHAJW-0KOArbX4JF2hYr4YC_avAeefEn5W-K5Hs": true, "kWsOfSN4pMBZ1e_dpSSdLI1OYJO7cdlIXPcWb6VzG9I": false, "VtypEh3UsoL0or6oMLs1ZpKJ6HlRxe5wuv4h3rcQZCw": false, "IB_es-lWOM11tdmC8Gk8v9Chh2oqm06PDdtM9SCiJtE": true, "KCLd60lBNScu7W0HdW6cWcTEHRyCHQ3mZGlk87eumm0": true, "kfhpWWU76-RAHOr_ClQFlsVkjxgNiQekKiv7aMwjExQ": true, "ii05XHdVV25PLA2Ab2tDyHE_rTrq1U7Vbq5Bx6Nowt0": true, "_unSjG217YkD92Ca-2plZc758lujjxbdfZ6VFT9lY4w": true, "6npCpEpMg2rde8LdW0zvCxZ5OU6JwyZIolu3Jsr1iKA": true, "lck4dFEV2WMu1z-fNb24McuNuSkN4Wxqzm9c0QjQQSg": true, "IZgRXnlSMnM4lH7NNDF8feJi5-Jl_-BW-EJ4a6o7hiQ": true, "iUuSZ8lV2eQFj_lNNxmW09sUMJKATF61BnSd_0y5hSc": true, "fdr72neBLxgx_9rOEFbgvOHA5u_MXWrZQzEHGZeVOWw": true, "IWVGvXgVhp6dtMCnUfGHYrV-V3R5-b26FlbWrr6nUeo": true, "luFHk0av3roDbMu6Ub7jc3tl43KXkaIeBWE2u2GhNLE": true },

NODE Validities: "validity": { "IC46XQwIu19jFhsKCsE_q5XUNAnUykCYpbp64Hp6Ueg": false, "rY5HBPNHn2qNEEjT7JTxgCI3RBqEKcicfhXSdzBgs3I": false, "Hg6oXHAJW-0KOArbX4JF2hYr4YC_avAeefEn5W-K5Hs": true, "kWsOfSN4pMBZ1e_dpSSdLI1OYJO7cdlIXPcWb6VzG9I": false, "VtypEh3UsoL0or6oMLs1ZpKJ6HlRxe5wuv4h3rcQZCw": false, "IB_es-lWOM11tdmC8Gk8v9Chh2oqm06PDdtM9SCiJtE": true, "KCLd60lBNScu7W0HdW6cWcTEHRyCHQ3mZGlk87eumm0": true, "kfhpWWU76-RAHOr_ClQFlsVkjxgNiQekKiv7aMwjExQ": true, "ii05XHdVV25PLA2Ab2tDyHE_rTrq1U7Vbq5Bx6Nowt0": true, "_unSjG217YkD92Ca-2plZc758lujjxbdfZ6VFT9lY4w": true, "6npCpEpMg2rde8LdW0zvCxZ5OU6JwyZIolu3Jsr1iKA": true, "lck4dFEV2WMu1z-fNb24McuNuSkN4Wxqzm9c0QjQQSg": true, "fdr72neBLxgx_9rOEFbgvOHA5u_MXWrZQzEHGZeVOWw": true, "IWVGvXgVhp6dtMCnUfGHYrV-V3R5-b26FlbWrr6nUeo": true, "luFHk0av3roDbMu6Ub7jc3tl43KXkaIeBWE2u2GhNLE": true },

As you can see the NODE version is missing 2 interactions.

Desktop (please complete the following information):

Additional Info Tried various evaluation options. This is what I use on the js side. I've tried just the defaults in Node and these as well. const contract = warp.contract(contractId).setEvaluationOptions({ allowBigInt: true, internalWrites: true, unsafeClient: "skip" });

arcj0 commented 1 year ago

Just read the same contract using node and warp-contracts version 1.2.56 and found that it has 2 more interactions than the JS version above. The JS version contains 17 interactions and the older 1.2.56 version contains 19 interactions.

ppedziwiatr commented 1 year ago

@arcj0 , can you please share your code on the frotnetnd side?

ppedziwiatr commented 1 year ago

So the results for the current version (from the main branch) are:

{
  IC46XQwIu19jFhsKCsE_q5XUNAnUykCYpbp64Hp6Ueg: false,
  rY5HBPNHn2qNEEjT7JTxgCI3RBqEKcicfhXSdzBgs3I: false,
  'Hg6oXHAJW-0KOArbX4JF2hYr4YC_avAeefEn5W-K5Hs': true,
  kWsOfSN4pMBZ1e_dpSSdLI1OYJO7cdlIXPcWb6VzG9I: false,
  VtypEh3UsoL0or6oMLs1ZpKJ6HlRxe5wuv4h3rcQZCw: false,
  'IB_es-lWOM11tdmC8Gk8v9Chh2oqm06PDdtM9SCiJtE': true,
  KCLd60lBNScu7W0HdW6cWcTEHRyCHQ3mZGlk87eumm0: true,
  'kfhpWWU76-RAHOr_ClQFlsVkjxgNiQekKiv7aMwjExQ': true,
  ii05XHdVV25PLA2Ab2tDyHE_rTrq1U7Vbq5Bx6Nowt0: true,
  '_unSjG217YkD92Ca-2plZc758lujjxbdfZ6VFT9lY4w': true,
  '6npCpEpMg2rde8LdW0zvCxZ5OU6JwyZIolu3Jsr1iKA': true,
  'lck4dFEV2WMu1z-fNb24McuNuSkN4Wxqzm9c0QjQQSg': true,
  'IZgRXnlSMnM4lH7NNDF8feJi5-Jl_-BW-EJ4a6o7hiQ': true,
  iUuSZ8lV2eQFj_lNNxmW09sUMJKATF61BnSd_0y5hSc: true,
  'rdtQEiLHzUkKCB4Ba8o0vdaVFMjvUogO7qCsToFHz-8': true,
  N2NEGXaerOjGs110KL_he3AKfvXgxOylE7Vx6xk0Dgg: true,
  fdr72neBLxgx_9rOEFbgvOHA5u_MXWrZQzEHGZeVOWw: true,
  'IWVGvXgVhp6dtMCnUfGHYrV-V3R5-b26FlbWrr6nUeo': true,
  luFHk0av3roDbMu6Ub7jc3tl43KXkaIeBWE2u2GhNLE: true,
  'fR-lblMVrDaYRKV_Q3mK6hdNjT6ql80-qg_E6JFGYUc': true
}

Read code:

const warp = WarpFactory
    .forTestnet({ ...defaultCacheOptions, inMemory: true });

  try {
    const contract = warp
      .contract("t5VFXv-YC4c4U5JhAk6kChQE8XArkj44DIbOh1kCvj8")
      .setEvaluationOptions({
        allowBigInt: true,
        unsafeClient: 'skip',
      });
    const result = await contract.readState();
    console.dir(result.cachedValue.validity, {depth: null});
  } catch (e) {
    console.error(e);
  }

At the time of writing - there are exactly 20 interaction on the validity list - exactly the same amount of interaction is presented by SonAR:

image

Again - can you please both your backend (i.e. 'node') and frontend code? ideally via a gh repo.

arcj0 commented 1 year ago

Thank you!

This seems to be working by using const warp = WarpFactory.forTestnet({ ...defaultCacheOptions, inMemory: true });

Before I was using const warp = WarpFactory.forTestnet().use(new DeployPlugin());

The eval options were the same. What's the difference?

arcj0 commented 1 year ago

Using the new setting, contracts are taking minutes to evaluate.

When I try to add remoteStateSyncEnabled: true, I get the LEVEL_INVALID_KEY error (in node). Is this because the contract source is blacklisted?

ppedziwiatr commented 1 year ago

Thank you!

This seems to be working by using const warp = WarpFactory.forTestnet({ ...defaultCacheOptions, inMemory: true });

Before I was using const warp = WarpFactory.forTestnet().use(new DeployPlugin());

The eval options were the same. What's the difference?

The only difference is that you're using 'in-memory' cache (inMemory: true) - this means, that the state is not loaded from db files and is not persisted and the end of script (it stays only in memory for the 'lifetime' of the script) and with each script run is evaluated from scratch. This is in general the equivalent of manually removing all the cache files after each script run.

arcj0 commented 1 year ago

Using this contract as an example: 6BZMY6p0Ub2lq-4ng6QwKLOvzyHd5ZrQwERSO-vNr6U

Using const warp = WarpFactory.forMainnet().use(new DeployPlugin()); I get the correct result (I think).

Using const warp = WarpFactory.forMainnet({ ...defaultCacheOptions, inMemory: true }); I get the original state.

Also, the evaluated state on Sonar is incorrect.

Can you tell me what settings I should be using to ensure we get the latest state of the contract?

I'm seeing these results in node and in js.

arcj0 commented 1 year ago

We're still seeing issues with different results when reading contracts. Just this morning, using the same contract as above (6BZMY6p0Ub2lq-4ng6QwKLOvzyHd5ZrQwERSO-vNr6U).

Sonar has the oldest version (Othent seems to match Sonar). Evaluating in Node a newer version shows. Our frontend seems to show the latest version.

NUMBER OF VALIDITIES: "WARP: 85" -> Read from node "FRONTEND: 82" -> Read from frontend "DEFAULT: 108" -> Read from node using defaultCacheOptions, inMemory: true (using this option is too slow)

Sonar -> really old version of state (on multiple DRE nodes)

arcj0 commented 1 year ago

Repo If I turn off remoteStateSyncEnabled and enable inMemory: true, the contract evaluates to 108 interactions which is correct, BUT it's WAY too slow for a production system.

ppedziwiatr commented 1 year ago

Hey! Sorry for the delayed response, had to take few days off.

So I've forked your repo and reevaluated the state from scratch locally in node - for both the default cache implementation (LevelDB) and sqlite (https://github.com/warp-contracts/warp-contracts-sqlite)

warp = WarpFactory
      .forMainnet({ ...defaultCacheOptions })
      .useStateCache(
        new SqliteContractCache(
          {
            ...defaultCacheOptions,
            dbLocation: `./cache/warp/sqlite/state`
          },
          {
            maxEntriesPerContract: 1000
          }
        )
      );

For both cases - exactly 131 interactions were evaluated - which is also what SonAR is showing as the amount of interactions for this contract

image

At the time of writing - the last evaluated interaction is https://sonar.warp.cc/#/app/interaction/93jG9qxw2-kn8J9XTtLUace2QH1a39ZgIVr_2Vl_90I?network=mainnet with sortKey = 000001255953,1694009078445,0cefe869555bd3436401faf1772dbcba2d739fd3d88f879fe94a44858bb59715 (this sortKey is also cached by warp - see next screenshot)

Each consecutive read results in simply loading the state from cache - as there are now no new interactions to process

image

So - I don't see any issues here.

I admit that there is something weird happenning on DRE nodes with this contract (and obviously if you're syncing from those nodes, you'll get different validity results). I've also manually 'asked' the DRE-1 to resync this contract (via the /sync endpoint).

What bothers me here is that it now it returns more than 131 interactions in the validity report (132): https://dre-1.warp.cc/contract?id=6BZMY6p0Ub2lq-4ng6QwKLOvzyHd5ZrQwERSO-vNr6U&validity=true&state=false

There is at least one 'weird' looking interaction on the above validity report (which is not recorded when evaluating the state locally from scratch): a9EeHwmCw9IJ9RXaQ6NVRLqN7zaaeLMDzltv1enkPVc - https://sonar.warp.cc/#/app/interaction/a9EeHwmCw9IJ9RXaQ6NVRLqN7zaaeLMDzltv1enkPVc?network=mainnet

This interaction has neither:

  1. 6BZMY6p0Ub2lq-4ng6QwKLOvzyHd5ZrQwERSO-vNr6U set in Contract tag, nor
  2. 6BZMY6p0Ub2lq-4ng6QwKLOvzyHd5ZrQwERSO-vNr6U set in Interact-Write tags (*)

Because the 6BZMY6p0Ub2lq-4ng6QwKLOvzyHd5ZrQwERSO-vNr6U is not set for none of the above tags (i.e. Contract tag or Interact-Write tag), the interaction loading mechanism simply does not know that it should load the a9EeHwmCw9IJ9RXaQ6NVRLqN7zaaeLMDzltv1enkPVc transaction for the 6BZMY6p0Ub2lq-4ng6QwKLOvzyHd5ZrQwERSO-vNr6U contract.

So - can you please explain a bit more what exactly this interaction does and what value is encoded in its input field?

image

If such interaction makes writes to other contracts - it MUST be somehow manifested in either the Contract tag or Interact-Write tag - otherwise - when lazily evaluating the state of the 6BZMY6p0Ub2lq-4ng6QwKLOvzyHd5ZrQwERSO-vNr6U, Warp will simply not know that it should load the a9EeHwmCw9IJ9RXaQ6NVRLqN7zaaeLMDzltv1enkPVc interaction for the 6BZMY6p0Ub2lq-4ng6QwKLOvzyHd5ZrQwERSO-vNr6U contract.

We can create some dedicated DRE instances for your set of contracts (just like it has been done for the Bazaar project and 'U' contract (https://dre-u.warp.cc/status) - whitelisting is performed based on the source tx ids) - BUT we need to first explain the issue with this 'othent.io' based interaction.

(*) - explanation - an example of an interaction with 6BZMY6p0Ub2lq-4ng6QwKLOvzyHd5ZrQwERSO-vNr6U set in Interact-Write - https://sonar.warp.cc/#/app/interaction/93jG9qxw2-kn8J9XTtLUace2QH1a39ZgIVr_2Vl_90I?network=mainnet - it means that (for the interaction from the link) the VyWHqrbuESTpry49YBL7l3UPiyE3ASvuWrOFXXbczJQ contract (which is set in the Contract tag) makes writes on both the 6BZMY6p0Ub2lq-4ng6QwKLOvzyHd5ZrQwERSO-vNr6U and UV6jzQNHAvalCMjRJIDahMBEF4fehMwZ7qDChHqaOhc contracts

arcj0 commented 1 year ago

I've asked Lorimer to respond regarding the interaction question as it's an Othent interaction.

I'm seeing similar issues when I'm not using Othent as well. Using the above contract, I just read the contract using node and only see 96 interactions (using defaultCacheOptions).

Here's another new contract with no Othent interactions and our systems are having similar issues. This contract is a different source. ContractID: -eWuM6nv1yABf5PxVJm75SkUAsXGl-S_LXOOlUL37bY

Read using defaultCacheOptions in Node - 4 interactions. (using SQL Lite I get 6 interactions) Read from web using defaultCacheOptions - 6 interactions (same on Sonar)

We have different sources reading our contracts as well as different users on this transactional system. So, I need to ensure that they're all getting the same results. Should we be using the sqllite?

arcj0 commented 1 year ago

Just trying to provide as much info as possible so we can get to the bottom of it. Here's another contract where I just made an interaction and I don't see it at all on Sonar: KvgS1Ikrc_gnveocS_YMgRmBGH5eic61dFXKWpSzjMw (I'm assuming this is because of the DRE sync you mentioned above).

I'm also seeing 2 different results when I read the contract on the web vs. in node using the sqlite settings you gave me above.

NODE (using sqlite settings):

  "validity": {
      "RCrrFTVrj7Ibnsx_g3U1oUXoM4eSZ5w7xEdFgvBcUOY": false,
      "ZyzblcN1D2fVND4COLEr05S7Uce-bknVsWgbYQzGrLY": true,
      "iRp2xW2lC1F4GYsMi8Zx6ENsgYtzoP-BZYTcmneo9SM": true
  }

WEB (using warp = WarpFactory.forMainnet().use(new DeployPlugin());)

  "validity": {
      "MqPojGtS5ZAQFgGsubeT6xPaLXgdi-YSB7U3GR37BYE": true,
      "UPd_Tq3bPXiLAz5xUr2PTEcwpa2bXS8bzk_Ep999j9Y": true,
      "iRp2xW2lC1F4GYsMi8Zx6ENsgYtzoP-BZYTcmneo9SM": true,
      "ZyzblcN1D2fVND4COLEr05S7Uce-bknVsWgbYQzGrLY": true,
      "RCrrFTVrj7Ibnsx_g3U1oUXoM4eSZ5w7xEdFgvBcUOY": true
  }

The Web version is correct. Othent wasn't involved in any of these interactions.

LorimerJenkins commented 1 year ago

Hey @ppedziwiatr, the interaction from othent is a transaction to the users smart contract wallet which then makes a internal write to another contract wherever the user desires.

You can decode the encoded payload on jwt.io and learn more about how othent works at docs.othent.io :)

arcj0 commented 1 year ago

I think we need to remove Othent from the equation right now until we figure out why we're seeing different versions of the source. Which is why I supplied more contract Ids as an example.

ppedziwiatr commented 1 year ago

KvgS1Ikrc_gnveocS_YMgRmBGH5eic61dFXKWpSzjMw

Are you sure that you have internal writes enabled in both cases? It looks as if the NODE version in your case was evaluating only the direct transactions - both the MqPojGtS5ZAQFgGsubeT6xPaLXgdi-YSB7U3GR37BYE and UPd_Tq3bPXiLAz5xUr2PTEcwpa2bXS8bzk_Ep999j9Y interactions are internal writes.

Can you please supply the 'WEB' version - I'm not seeing it in the https://github.com/aftrmarket/warp-contract-test repo?

ppedziwiatr commented 1 year ago

Hey @ppedziwiatr, the interaction from othent is a transaction to the users smart contract wallet which then makes a internal write to another contract wherever the user desires.

You can decode the encoded payload on jwt.io and learn more about how othent works at docs.othent.io :)

hmm, ok, but if there's is no 'trace' of such call in the tags, it won't be possible to lazily evaluate from scratch the contracts called by the "users smart contract wallet" properly. But as @arcj0 have mentioned, let's not mix two different issues.

As an example - the mentioned https://sonar.warp.cc/#/app/interaction/a9EeHwmCw9IJ9RXaQ6NVRLqN7zaaeLMDzltv1enkPVc?network=mainnet transaction should have in this case an Interact-Write tag with 6BZMY6p0Ub2lq-4ng6QwKLOvzyHd5ZrQwERSO-vNr6U value.

arcj0 commented 1 year ago

I always have internalWrites set to true.

I had a web test repo already set up (just needed to modify to focus on this issue). https://github.com/aftrmarket/vue-contract-test

If I use inMemory: true in both Node and Vue I get correct results. Problem is that it's way too slow for just a handful of interactions.

Note that the contract error is from our testing of the contract and is a legitimate error where the contract catches an invalid interaction attempt.

ppedziwiatr commented 1 year ago

What really would help is an integration test with a MRE. With the KvgS1Ikrc_gnveocS_YMgRmBGH5eic61dFXKWpSzjMw contract, even the very first interaction (which is an internal write) generates a lot of read/writes during its evaluation (~45 contracts are being cached after evaluating this interaction, the call depth is up to 6 AFAIR) - I'm afraid that analysis of this is beyond my little brain capacity.

So - an intergration test with a MRE would be really helpful here! (sth similar to the test that I've created a while ago for another issue that you've raised - https://github.com/warp-contracts/warp/blob/main/src/__tests__/integration/internal-writes/internal-write-aftr.test.ts , which turned out to be a missing await in the contract code and/or a read after internal write (instead of using result from the write directly)).

Thank you!

In the meantime - we (i.e. me and @Tadeuchi ) will continue to analyse the 'real-life' examples.

arcj0 commented 1 year ago

I'll try to put together an MRE. The problem is that I don't know which example to give you. There are several platform transactions at play here as you can see from the contract communication through the internal writes. I'll work on it.

In the meantime, I noticed contract 9SyDYB3muSvJh12vHO4zKz4dOJyCwT2ZZyQ9bziag60 has missing interactions values: When reading using inMemory: true

{
  "EqFi53tulzp3Rl5yPwLenh8pNVquO-TdsbxgBBNBu0o":,        // NO VALUE
  "Iv-Hy3XCWkqBigqjY2jVguY-Y8Lt-S2bAwEyqFBEqe8":,        // NO VALUE
  "a5Ap25jOOn7Wlle7eFtHVtgCwYf27XHBzWBoznnhIR0": true,
  "-d2S19dV5OXMvPbBZiPvbvlMz-C_oSbOPVSak00qYps": false,
  "9bV1R7yD3oQyhWOCTwrYwju-uWL50cAwUurb5hVjByg": false,
  "CPzZ3tUgfhbzfWFuNlbI-9sX9Vyee6Ne4P1bckrKRoY": false,
  "ThAvju2IZBMlrLpzUHvlvpeDlA1O3KT5u0ibQGKDt3o": false
}

There are not values in the first 2 interactions. Because of this, interactions 3 - 7 fail.

But, if I don't read with the inMemory option, these results are as follows:

{
  "EqFi53tulzp3Rl5yPwLenh8pNVquO-TdsbxgBBNBu0o": true,
  "Iv-Hy3XCWkqBigqjY2jVguY-Y8Lt-S2bAwEyqFBEqe8": true,
  "a5Ap25jOOn7Wlle7eFtHVtgCwYf27XHBzWBoznnhIR0": true,
  "-d2S19dV5OXMvPbBZiPvbvlMz-C_oSbOPVSak00qYps": true,
  "9bV1R7yD3oQyhWOCTwrYwju-uWL50cAwUurb5hVjByg": true,
  "CPzZ3tUgfhbzfWFuNlbI-9sX9Vyee6Ne4P1bckrKRoY": true,
  "ThAvju2IZBMlrLpzUHvlvpeDlA1O3KT5u0ibQGKDt3o": true
}

I'm not sure if that can help clue us in as to what is going on, but I can tell you that these interactions are a part of an internal write that is making a claim as a 2nd part of an FCP call (the claim of the allow / claim sequence). Why would reading them in one way produce neither a true or false value?

I also don't want to assume that this is the only problem because I've given examples of other contract sources that are experiencing similar issues with different state evaluations.

ppedziwiatr commented 1 year ago

@arcj0 :

  1. which SDK version is exactly being used in the 'WEB' version?

  2. which SDK version was used for evaluating the contract without 'inMemory' option? I.e. in case of the validity report that you've pasted (without using the 'inMemory' option):

    {
    "EqFi53tulzp3Rl5yPwLenh8pNVquO-TdsbxgBBNBu0o": true,
    "Iv-Hy3XCWkqBigqjY2jVguY-Y8Lt-S2bAwEyqFBEqe8": true,
    "a5Ap25jOOn7Wlle7eFtHVtgCwYf27XHBzWBoznnhIR0": true,
    "-d2S19dV5OXMvPbBZiPvbvlMz-C_oSbOPVSak00qYps": true,
    "9bV1R7yD3oQyhWOCTwrYwju-uWL50cAwUurb5hVjByg": true,
    "CPzZ3tUgfhbzfWFuNlbI-9sX9Vyee6Ne4P1bckrKRoY": true,
    "ThAvju2IZBMlrLpzUHvlvpeDlA1O3KT5u0ibQGKDt3o": true
    }
arcj0 commented 1 year ago

I'll check, but I think I'm on the same version for all of them.

I might have found something! I now have several cases of this same situation where the interactions are missing the t/f value. And so far, they're all the same interaction! I think I can now replicate this.

ppedziwiatr commented 1 year ago

Also, in case of contract 9SyDYB3muSvJh12vHO4zKz4dOJyCwT2ZZyQ9bziag60 and interaction EqFi53tulzp3Rl5yPwLenh8pNVquO-TdsbxgBBNBu0o - can you show any 'proof' that validity for this interaction should indeed be true?

I've evaluated this contract only for this one interaction - at its sortKey (000001259985,1694524397575,31d4eb8fd0fa0414e7d7d7feaf39f56e03d6ace1413f1e545a01462238871e62), ie. by calling:

const result = await contract.readState("000001259985,1694524397575,31d4eb8fd0fa0414e7d7d7feaf39f56e03d6ace1413f1e545a01462238871e62");

and the generated call stack (which can be obtained via contract.getCallStack().print()) is HUGE (attaching in zip file). The call stack is basically a trace of all the calls made during evaluation of each of the contract interaction - in a tree structure - you can see, that only EqFi53tulzp3Rl5yPwLenh8pNVquO-TdsbxgBBNBu0o interaction of the 9SyDYB3muSvJh12vHO4zKz4dOJyCwT2ZZyQ9bziag60 contract is being evaluated:

image

and it starts with a read from the EJH0Oi7_-ZCvjz0nZAWS-QsiHFOPZnE-u26c_dKROUY contract

image

...and then it goes wild... some of the interactions of the EJH0Oi7_-ZCvjz0nZAWS-QsiHFOPZnE-u26c_dKROUY contract (that are being evaluated up to the requested sortKey - 15 interaction in total) make views or writes to other contracts (which often fail - eg the below example - interaction Laf3hTJnayV9fruCE0hV9S1dXVGoza-Mq8bIobuvZVk of the EJH0Oi7_-ZCvjz0nZAWS-QsiHFOPZnE-u26c_dKROUY contract makes a 'write' to the XlXYsmQFW0LA7WkmqfplDlzEGCROGjLTUopeWIcn3rM contract and it fails with "errorMessage": "There must be 1 claimable with this Tx ID."

image

callStack.json.zip

ppedziwiatr commented 1 year ago

I'll check, but I think I'm on the same version for all of them.

I might have found something! I now have several cases of this same situation where the interactions are missing the t/f value. And so far, they're all the same interaction! I think I can now replicate this.

Yup, we're tracing this bug, there's apparently some issue with setting validity when 'view' functions are being called along the process...

Though I'm still not sure how did you get the 'true' for the first interaction...the current versions of the SDK always produce 'undefined' (that's what we will try to fix).

The older SDK versions (I've tested on 1.2.58, 1.2.48, 1.2.33) produce 'false' - no matter what is the cache implementation, etc.

arcj0 commented 1 year ago

Reading your previous message makes me concerned about the architecture here. Basically, these contracts are passing assets back and forth. So, they will be a lot interactions, and this is only a SMALL example. We might have to rethink the architecture if this proves to be too complex. If that's the case, that concerns me about using this as a transaction system.

arcj0 commented 1 year ago

I'm checking all the places we use viewContractState. We use that call in 3 of our contracts. I'll focus the MRE on this.

ppedziwiatr commented 1 year ago

We're still making our tests and I believe we've spotted at least on issue in the contract code - missing await. I got this error

image

during the evaluation of the KvgS1Ikrc_gnveocS_YMgRmBGH5eic61dFXKWpSzjMw contract.

It looks really strange, cause the

undefined:357
      if (participants[i].teamScore < participant.teamScore) {
                          ^

TypeError: Cannot read properties of undefined (reading 'teamScore')

seems to be thrown 'out' of the main event loop (and it is often a sign of a missing 'await' somewhere in the code).

Now, reviewing the KvgS1Ikrc_gnveocS_YMgRmBGH5eic61dFXKWpSzjMw contract source code:

  1. The if (participants[i].teamScore < participant.teamScore) { code is here: image

- inside the assignTiers function. This funtion is sync, BUT it is called by the async function distributeRewards:

image
  1. Going further, the 'distributeRewards' is called inside the async function finalizeGame:
image

- BUT without await - i.e. I believe that the code in line 427 should be await distributeRewards(state); (OR remove the 'async' from the distributeRewards as it doesn't seem to make any async calls?)

side note: not sure if error handling in line 424 is the best possible - i.e. the:

if (response.type !== "ok") {
}
arcj0 commented 1 year ago

ugh, thx I'll check that! Actually, I think I know how that happened. distributeRewards used to make async calls (assignTiers was async before we changed it). When we started seeing issues, we removed that functionality and made it a separate contract interaction.

Regarding the contract errors, in some cases we don't want to throw a contract error b/c it will cancel the interaction all together when we need it to complete. With that said though, we have rearchitected (as I mentioned above) so that contracts don't have to "talk" to a bunch of other contracts in one interaction (if that makes sense).

ppedziwiatr commented 1 year ago

Yup, the simpler architecture, the better. In the meantime, we will try to fix the issues with setting validation in certain scenarios.

Re. error handling..well, the next line after this line is:

createHistory(state, response.result.playerScores);
arcj0 commented 1 year ago

As I was working on an MRE, I found this which might be helpful to you because it's the first interaction on the contract. This should answer your question above about how to get the interaction to evaluate to true when the one without the cache is undefined.

I created new contracts replacing the viewContractStates with readContracts (there is 1 viewContractState left, but it's not in code that's being called). I was trying to see if the viewContractStates were the source of the issues.

Interestingly, I was able to replicate interaction value undefined behavor with I called the joinGame interaction on this contract. The joinGame interaction is called from another contract as an internalWrite (it's done this way for several reasons, but that's another rabbit hole). Now, if you read the contract using no cache (inMemory: true), you get the interaction without a value just like before. But, if you read it with default options (cache), then the interaction is true. I even cleared my browser cache, ran it with inMemory: true, then switched back to default and get the same behavior where the interaction is true. So it's consistent.

New Contract: xm7ZspA2EG55Qo7rUf2UCPgYVdlotdTd_HIaG9KukzE

default (browser cache)

"validity": {
  "ux8QdqxAO5xQTvZKyQ1GSrlxgb4KyQbKRyzpIfATwZo": true
}

inMemory: true

"validity": {
  "ux8QdqxAO5xQTvZKyQ1GSrlxgb4KyQbKRyzpIfATwZo": 
}

I hope this helps! I did create an MRE, but I couldn't replicate the behavior we're looking for. I discovered this when I was testing the new contract deployments on the frontend.

arcj0 commented 1 year ago

I think I may have replicated it with this example. The script runs a sequence of interactions with 3 contracts and then when you read the 3rd contract with and without cache, you get 2 different results.

Example is here.

ppedziwiatr commented 1 year ago

Thank you @arcj0 , we will analyse your example!

ppedziwiatr commented 1 year ago

So I've run your latest example and got the same results:

image

After pasting the results to IDE and comparing them:

image

'Contents are identical'....

Also pasting my output from your script (i.e. https://github.com/aftrmarket/warp-contract-test/blob/main/scripts/test-purchase.js#L161-L162):

{"sortKey":"000001263979,1695042688644,b3ca7f33402207721fd70745f996738a6e43fca92ddea94139e74821d742ab51","cachedValue":{"state":{"name":"Ugly Ducks","games":[],"owner":"bAJYgxGXt9KE4g8H7l7u80iFaBIgzpUQNUgycJby0lU","votes":[],"claims":[],"status":"started","ticker":"UDUCKS","tokens":[{"txID":"kuK55QPYogdzGyF1xT16ZITfMuo3_pq7l4s0w253mMU","tokenId":"fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54","source":"bAJYgxGXt9KE4g8H7l7u80iFaBIgzpUQNUgycJby0lU","balance":1,"start":"1263692","name":"Testy Tester","ticker":"TESTER-TESTY","logo":"","lockLength":0},{"txID":"hl78akn8MFWiHZK2ZQQLNF0-aAY6kx8EDTV1b4eLnnQ","tokenId":"fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54","source":"bAJYgxGXt9KE4g8H7l7u80iFaBIgzpUQNUgycJby0lU","balance":1,"start":"1263979","name":"Testy Tester","ticker":"TESTER-TESTY","logo":"","lockLength":0}],"balances":{"bAJYgxGXt9KE4g8H7l7u80iFaBIgzpUQNUgycJby0lU":1},"settings":[["quorum",0.5],["support",0.51],["voteLength",2160],["communityLogo",""]],"claimable":[],"ownership":"single","protected":["teamSetup","teamLimits","games"],"teamSetup":{"bench":[{"id":"fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54","position":"wr","orgId":"CLEM"},{"id":"fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54","position":"wr","orgId":"CLEM"}],"starters":[]},"teamLimits":[{"position":"qb","starterLimit":1,"positionLimit":2},{"position":"rb","starterLimit":2,"positionLimit":4},{"position":"wr","starterLimit":3,"positionLimit":100},{"position":"k","starterLimit":1,"positionLimit":2},{"position":"d","starterLimit":1,"positionLimit":2}],"votingSystem":"weighted"},"validity":{"ab0TUtwNOw-fZv5CRsmyU0ZXWXfR4Z9JOnh3gtq730c":false,"xfKCHynz8y_UJwvNV_oE9ihLmProtfLxry6ZwmIgL2Y":true,"VSNkX9fbYorqefITZ27_1DqTbNM16N94U95XYn69aXw":true},"errorMessages":{"ab0TUtwNOw-fZv5CRsmyU0ZXWXfR4Z9JOnh3gtq730c":"Internal write auto error for call [{\"from\":\"H1doqUpcNYDcoQiWYcP4plGOzfWRbwObdY7cyNuh_Wc\",\"to\":\"fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54\",\"input\":{\"function\":\"reject\",\"tx\":\"Ix3JtFXt5KT5_ldBvioYNgIFMGZjkZtkF2UjgKrvU0c\"}}]: There must be 1 claimable with this Tx ID."}}}
{"sortKey":"000001263979,1695042688644,b3ca7f33402207721fd70745f996738a6e43fca92ddea94139e74821d742ab51","cachedValue":{"state":{"name":"Ugly Ducks","games":[],"owner":"bAJYgxGXt9KE4g8H7l7u80iFaBIgzpUQNUgycJby0lU","votes":[],"claims":[],"status":"started","ticker":"UDUCKS","tokens":[{"txID":"kuK55QPYogdzGyF1xT16ZITfMuo3_pq7l4s0w253mMU","tokenId":"fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54","source":"bAJYgxGXt9KE4g8H7l7u80iFaBIgzpUQNUgycJby0lU","balance":1,"start":"1263692","name":"Testy Tester","ticker":"TESTER-TESTY","logo":"","lockLength":0},{"txID":"hl78akn8MFWiHZK2ZQQLNF0-aAY6kx8EDTV1b4eLnnQ","tokenId":"fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54","source":"bAJYgxGXt9KE4g8H7l7u80iFaBIgzpUQNUgycJby0lU","balance":1,"start":"1263979","name":"Testy Tester","ticker":"TESTER-TESTY","logo":"","lockLength":0}],"balances":{"bAJYgxGXt9KE4g8H7l7u80iFaBIgzpUQNUgycJby0lU":1},"settings":[["quorum",0.5],["support",0.51],["voteLength",2160],["communityLogo",""]],"claimable":[],"ownership":"single","protected":["teamSetup","teamLimits","games"],"teamSetup":{"bench":[{"id":"fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54","position":"wr","orgId":"CLEM"},{"id":"fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54","position":"wr","orgId":"CLEM"}],"starters":[]},"teamLimits":[{"position":"qb","starterLimit":1,"positionLimit":2},{"position":"rb","starterLimit":2,"positionLimit":4},{"position":"wr","starterLimit":3,"positionLimit":100},{"position":"k","starterLimit":1,"positionLimit":2},{"position":"d","starterLimit":1,"positionLimit":2}],"votingSystem":"weighted"},"validity":{"ab0TUtwNOw-fZv5CRsmyU0ZXWXfR4Z9JOnh3gtq730c":false,"xfKCHynz8y_UJwvNV_oE9ihLmProtfLxry6ZwmIgL2Y":true,"VSNkX9fbYorqefITZ27_1DqTbNM16N94U95XYn69aXw":true},"errorMessages":{"ab0TUtwNOw-fZv5CRsmyU0ZXWXfR4Z9JOnh3gtq730c":"Internal write auto error for call [{\"from\":\"H1doqUpcNYDcoQiWYcP4plGOzfWRbwObdY7cyNuh_Wc\",\"to\":\"fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54\",\"input\":{\"function\":\"reject\",\"tx\":\"Ix3JtFXt5KT5_ldBvioYNgIFMGZjkZtkF2UjgKrvU0c\"}}]: There must be 1 claimable with this Tx ID."}}}
arcj0 commented 1 year ago

@ppedziwiatr - Please try it again. With the latest example, I can make it fail consistently on my end.

With Cache:

    "validity": {
      "ab0TUtwNOw-fZv5CRsmyU0ZXWXfR4Z9JOnh3gtq730c": true,
      "xfKCHynz8y_UJwvNV_oE9ihLmProtfLxry6ZwmIgL2Y": true,
      "VSNkX9fbYorqefITZ27_1DqTbNM16N94U95XYn69aXw": true,
      "3UKTNhTTjxH1R63L11hOtOlBaxrNOGYG2e7qBUgCHPQ": true,
      "wFbhXObB8wgInunOYVqJJlWFRayz8ovit70odtdlkmI": true,
      "Zhu293_eIW10XY60mhIj1okbwUkLnXA4QtaO48G78GU": true,
      "UpSZkZjWM3uCcKwaufqTRxntpJdq8gsUK_t6H1mnq3k": true,
      "QsQo01B49UO8HAf-JPKKKvTbxwURmx-wLI-g9ZDNeu8": true,
      "TfYjq2oqdr9AeEGq4PQT4YzdZ9Q6blepBBtj_Ngh5ac": true,
      "kxq4KCkOUjZ9FiHHFaGGFldOCFN-scJmZw7GCGlVSDM": true,
      "9_izuiMleptt0b5qjIR52sjqygfMOwcpQiTXr24WeAs": true,
      "keFKIhyuwYBM_FkTT2KIg9tR_Xl6ffR8gRdeXBNddLI": true,
      "SZyxegZ-D6hoAlX2eGr1FlMgIvd-HBirXMLwwDL0slY": true,
      "ED51rpOj3jBbTnoovFXUiTr1mTjDpIoJ55i5yON9P3w": true,
      "VK4bAbogCUP8Cm2ymNjGm11_G76JD-HtXqOJH1dooPo": true
    }

Without Cache

    "validity": {
      "ab0TUtwNOw-fZv5CRsmyU0ZXWXfR4Z9JOnh3gtq730c": false,
      "xfKCHynz8y_UJwvNV_oE9ihLmProtfLxry6ZwmIgL2Y": true,
      "VSNkX9fbYorqefITZ27_1DqTbNM16N94U95XYn69aXw": true,
      "3UKTNhTTjxH1R63L11hOtOlBaxrNOGYG2e7qBUgCHPQ": true,
      "wFbhXObB8wgInunOYVqJJlWFRayz8ovit70odtdlkmI": true,
      "Zhu293_eIW10XY60mhIj1okbwUkLnXA4QtaO48G78GU": true,
      "UpSZkZjWM3uCcKwaufqTRxntpJdq8gsUK_t6H1mnq3k": true,
      "QsQo01B49UO8HAf-JPKKKvTbxwURmx-wLI-g9ZDNeu8": true,
      "TfYjq2oqdr9AeEGq4PQT4YzdZ9Q6blepBBtj_Ngh5ac": true,
      "kxq4KCkOUjZ9FiHHFaGGFldOCFN-scJmZw7GCGlVSDM": true,
      "9_izuiMleptt0b5qjIR52sjqygfMOwcpQiTXr24WeAs": true,
      "keFKIhyuwYBM_FkTT2KIg9tR_Xl6ffR8gRdeXBNddLI": false,
      "SZyxegZ-D6hoAlX2eGr1FlMgIvd-HBirXMLwwDL0slY": false,
      "ED51rpOj3jBbTnoovFXUiTr1mTjDpIoJ55i5yON9P3w": false,
      "VK4bAbogCUP8Cm2ymNjGm11_G76JD-HtXqOJH1dooPo": false
    }

You should be able to see the same if you rerun the example.

arcj0 commented 1 year ago

A member from our team ran this same example and here are his results:

"validity": {
      "ab0TUtwNOw-fZv5CRsmyU0ZXWXfR4Z9JOnh3gtq730c": false,
      "xfKCHynz8y_UJwvNV_oE9ihLmProtfLxry6ZwmIgL2Y": true,
      "VSNkX9fbYorqefITZ27_1DqTbNM16N94U95XYn69aXw": true,
      "3UKTNhTTjxH1R63L11hOtOlBaxrNOGYG2e7qBUgCHPQ": true,
      "wFbhXObB8wgInunOYVqJJlWFRayz8ovit70odtdlkmI": true,
      "Zhu293_eIW10XY60mhIj1okbwUkLnXA4QtaO48G78GU": true,
      "UpSZkZjWM3uCcKwaufqTRxntpJdq8gsUK_t6H1mnq3k": true,
      "QsQo01B49UO8HAf-JPKKKvTbxwURmx-wLI-g9ZDNeu8": true,
      "TfYjq2oqdr9AeEGq4PQT4YzdZ9Q6blepBBtj_Ngh5ac": true,
      "kxq4KCkOUjZ9FiHHFaGGFldOCFN-scJmZw7GCGlVSDM": true,
      "9_izuiMleptt0b5qjIR52sjqygfMOwcpQiTXr24WeAs": true,
      "keFKIhyuwYBM_FkTT2KIg9tR_Xl6ffR8gRdeXBNddLI": true,
      "SZyxegZ-D6hoAlX2eGr1FlMgIvd-HBirXMLwwDL0slY": true,
      "ED51rpOj3jBbTnoovFXUiTr1mTjDpIoJ55i5yON9P3w": true,
      "VK4bAbogCUP8Cm2ymNjGm11_G76JD-HtXqOJH1dooPo": true,
      "IoqY2p7aPRpeJk3fYayV6AcqKiZfw9mGpEwRXFq-r04": true,
      "zwS9YqVxfCPK_7dKQtheyhELvVojNUumksoKI7aeIOU": true
    }
"validity": {
      "ab0TUtwNOw-fZv5CRsmyU0ZXWXfR4Z9JOnh3gtq730c": false,
      "xfKCHynz8y_UJwvNV_oE9ihLmProtfLxry6ZwmIgL2Y": false,
      "VSNkX9fbYorqefITZ27_1DqTbNM16N94U95XYn69aXw": false,
      "3UKTNhTTjxH1R63L11hOtOlBaxrNOGYG2e7qBUgCHPQ": false,
      "wFbhXObB8wgInunOYVqJJlWFRayz8ovit70odtdlkmI": false,
      "Zhu293_eIW10XY60mhIj1okbwUkLnXA4QtaO48G78GU": false,
      "UpSZkZjWM3uCcKwaufqTRxntpJdq8gsUK_t6H1mnq3k": false,
      "QsQo01B49UO8HAf-JPKKKvTbxwURmx-wLI-g9ZDNeu8": false,
      "TfYjq2oqdr9AeEGq4PQT4YzdZ9Q6blepBBtj_Ngh5ac": false,
      "kxq4KCkOUjZ9FiHHFaGGFldOCFN-scJmZw7GCGlVSDM": false,
      "9_izuiMleptt0b5qjIR52sjqygfMOwcpQiTXr24WeAs": false,
      "keFKIhyuwYBM_FkTT2KIg9tR_Xl6ffR8gRdeXBNddLI": false,
      "SZyxegZ-D6hoAlX2eGr1FlMgIvd-HBirXMLwwDL0slY": false,
      "ED51rpOj3jBbTnoovFXUiTr1mTjDpIoJ55i5yON9P3w": false,
      "VK4bAbogCUP8Cm2ymNjGm11_G76JD-HtXqOJH1dooPo": false,
      "IoqY2p7aPRpeJk3fYayV6AcqKiZfw9mGpEwRXFq-r04": false,
      "zwS9YqVxfCPK_7dKQtheyhELvVojNUumksoKI7aeIOU": false
    }

The odd thing here is that nothing without cache is true. How can we have different results from the blockchain?

ppedziwiatr commented 1 year ago

we're reviewing (and rewriting..) some internals of how the IW are functionining...please give us some time. In the meantime:

  1. we can prepare a dedicated DRE nodes for your dapp - similar to those dre-u nodes, that have a bit different way of processing transactions (which provides a consistent results - even with the current versions of the SDK)
  2. it would be great if your example could be really narrowed down to a minimal example, that can be run as an integration test with an arlocal. The current example seems to run on mainnet contracts, which keep on getting new interactions + it simply evaluates a lot of contracts in the process..

Answering your question - the 'blockchain' (ie. Arweave) stores only transactions. There's probably some issue with evaluating them with differnt versions of the SDK / cache options for your contracts/dapp.

As I've said - we're in the process of writing some of the internal parts of the SDK responsible for the internal contracts calls.

Thanks!

arcj0 commented 1 year ago

I'll work on an example. I don't think we've been able to replicate the issue locally and we were told to use mainnet instead of testnet because testnet may not be configured properly with dre.

ppedziwiatr commented 1 year ago

I've narrowed down the test to only the first interaction (i.e. I'm reading the state at the sortKey of the very first interaction with the contract - https://sonar.warp.cc/#/app/interaction/ab0TUtwNOw-fZv5CRsmyU0ZXWXfR4Z9JOnh3gtq730c?network=mainnet). Reading the current state makes it really impossible to understand what is going on, as the there are constantly new interactions being registered. Still - even reading for the first interaction generates a ton of internal calls (writes/reads).

Anyway - can you please run the test from my fork? https://github.com/ppedziwiatr/warp-contract-test

  1. verify warp-contracts version in node_modules (should be 1.4.18)
  2. rm -rf cache
  3. npm run test-purchase
  4. compare the results of the result_cache_false.json and callstack_cache_true.json (there should be NO difference and the validity of the interaction ab0TUtwNOw-fZv5CRsmyU0ZXWXfR4Z9JOnh3gtq730c should be false)
  5. compare the results of the callstack_cache_false.json and callstack_cache_true.json - the only differences should the values in the executionTime and the randomly assigned uuids.

I've also attached my results in the latest commit - https://github.com/ppedziwiatr/warp-contract-test/commit/e7ea00c8649bc4b9a31777163460ed89f6274569

ppedziwiatr commented 1 year ago

I'm also trying to understand what in fact is going on in this first interaction.

The errorMessage from the result suggests that the contract H1doqUpcNYDcoQiWYcP4plGOzfWRbwObdY7cyNuh_Wc is calling fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54 with a reject function (probably because the 'canClaim' returned 'false':

image

)

BUT:

  1. the canClaim reads the fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54 state and checks whether the txId from the input is on its 'claimable' list (plus some other stuff)
  2. the reject itself in the fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54 simply fails if the requested txId is not on the claimable list image

- and that's what is visible in the errorMessage:

"Internal write auto error for call [{\"from\":\"H1doqUpcNYDcoQiWYcP4plGOzfWRbwObdY7cyNuh_Wc\",\"to\":\"fT561zunBM60fSFG4lJ54x8M18H2jC682xU9pVSHG54\",\"input\":{\"function\":\"reject\",\"tx\":\"Ix3JtFXt5KT5_ldBvioYNgIFMGZjkZtkF2UjgKrvU0c\"}}]: There must be 1 claimable with this Tx ID."

This basically means that if canClaim will return false and the reject will be called, there is a high chance that the whole base interaction will fail.

Can you please show any proof that the validity of the ab0TUtwNOw-fZv5CRsmyU0ZXWXfR4Z9JOnh3gtq730c should be 'true'?

arcj0 commented 1 year ago

I've finally been able to figure out what's going on and have replicated the issue in an MRE. Please see the updated example. The README should have all the information you need. Let me know if you have any questions.

ppedziwiatr commented 1 year ago

Thanks! We will analyse your example and test any future fixes/updates with it (plus probably we will add it to our standard set of integration tests). We will keep you informed!

ppedziwiatr commented 1 year ago

@arcj0

image
ppedziwiatr commented 1 year ago

Also, I'm trying to get a bit more understanding of the whole app.

1.

Purchase the same player for 2 teams.

Player is purchased for Team 1 successfully. For Team 2, the Player is partially purchased as the 'canClaim' function is returning a false value. I removed the 'reject' internal write for the test so we can just focus on why the function is failing.

This behaviour is kinda intuitive for me - since the Player is already purchased for the Team 1, it is kinda natural the 'same' Player cannot be purchased for the Team 2.

Does your dApp assume the same Player can be purchased by multiple Teams at the same time?

2.

To easily see the issue, I added 2 objects to the state of the team contracts.

Can you please send exact links to the contract code where these objects are added?

Also - reviewing the contracts' code - player and team contracts seem to have hundreds lines of unused code? Those look like c-p from the 'community' contracts with some added functionality on top. Do we really need all the community-related code? it adds additional noise... The 'currency' contract looks like some fairly standard PST and the registry contract simply holds all the players, right?

arcj0 commented 1 year ago

Contributor

Sorry, was in the gitignore. It's there now.

ppedziwiatr commented 1 year ago

Also, moving forward:

The 2nd purchase of the Player by Team 2 is failing because Team 2 can't find the claim on the Player contract.

If I understand correctly - it is this https://github.com/ppedziwiatr/warp-contract-test/blob/main/__tests__/purchase.test.js#L128 function that is effecitvely failing (ie. the deposit for the team 2) - because the canClaim inside the deposit cannot find the claim in the player contract (and it should be there, because it was added by the allow function called on the player contract - i.e. this one https://github.com/ppedziwiatr/warp-contract-test/blob/main/__tests__/purchase.test.js#L119) Right?

arcj0 commented 1 year ago
  1. This behaviour is kinda intuitive for me - since the Player is already purchased for the Team 1, it is kinda natural the 'same' Player cannot be purchased for the Team 2.

Does your dApp assume the same Player can be purchased by multiple Teams at the same time?

Yes, that's correct. It's the same as multiple users having a balance on the same asset.

  1. Can you please send exact links to the contract code where these objects are added? Team Contract - Line 428 and Team Contract - Line 919.

I'm still responding, but I see that you're actively working on this, so sending this part now as it answers your first 2 questions.

arcj0 commented 1 year ago

If I understand correctly - it is this https://github.com/ppedziwiatr/warp-contract-test/blob/main/__tests__/purchase.test.js#L128 function that is effecitvely failing (ie. the deposit for the team 2) - because the canClaim inside the deposit cannot find the claim in the player contract (and it should be there, because it was added by the allow function called on the player contract - i.e. this one https://github.com/ppedziwiatr/warp-contract-test/blob/main/__tests__/purchase.test.js#L119) Right?

Yes, that's correct.

Also - reviewing the contracts' code - player and team contracts seem to have hundreds lines of unused code? Those look like c-p from the 'community' contracts with some added functionality on top. Do we really need all the community-related code? it adds additional noise... The 'currency' contract looks like some fairly standard PST and the registry contract simply holds all the players, right?

I used the contracts from the platform itself. I'll can try to stripe out everything that is not associated in this example if that helps. These contracts are slightly customized AFTR contracts which were based off the original Community XYZ contracts from a few years ago. And, yes the currency contract is a simplified PST contract. The registry contracts are used to hold player info as it changes through the application timeline. It's sorta like an on-chain DB used for quick validations and to prevent having to update 1000s of player tokens separately.

ppedziwiatr commented 1 year ago

@arcj0 can you please test on 1.4.19-beta.0

image

or use my fork directly - https://github.com/ppedziwiatr/warp-contract-test

arcj0 commented 1 year ago

@ppedziwiatr There's a dependency conflict, but I'm using --force to ignore it.

This appears to work. What changed?

ppedziwiatr commented 1 year ago

This appears to work. What changed?

https://github.com/warp-contracts/warp/pull/460

This commit specifically https://github.com/warp-contracts/warp/pull/460/commits/89d09c06e4a9cfde931478c4fbf3e57516e89f56

with additional tests: https://github.com/warp-contracts/warp/pull/460/commits/89d09c06e4a9cfde931478c4fbf3e57516e89f56#diff-732f6810a4fedadbe350403bbf094bedb242a017955b46eb42decdfd4b357181

arcj0 commented 1 year ago

@ppedziwiatr I'm assuming that we'll still see issues while using DRE nodes that don't have this fix, correct?