warp-contracts / warp

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

VRF issues with internal writes or "strict" mode. #232

Closed noomly closed 2 years ago

noomly commented 2 years ago

When using Vrf on mainnet, multiple errors TypeError: Cannot read properties of null (reading 'bigint') are logged when calling readState (if a previous interaction contains a call to Vrf) or writeInteraction, even if Vrf was enabled the interactions were successful.

Here is a quick example showcasing the behavior:

// const warp = WarpFactory.forTestnet(undefined, { inMemory: true, dbLocation: "/dev/null" });
// const user = await warp.testing.generateWallet();

const warp = WarpFactory.forMainnet({ inMemory: true, dbLocation: "/dev/null" });
const user = await generateWallet();

// const contractTxId = "gwQ-rKRo7GKX6vGEk1fXyILLdeO9vN4QfT97i3c2678"; // testnet
const contractTxId = "Ws9hhYckc-zSnVmbBep6q_kZD5zmzYzDmgMC50nMiuE"; // mainnet

const contract = warp
    .contract(contractTxId)
    .setEvaluationOptions({ internalWrites: false })
    .connect(user.jwk);

const txId = await contract.writeInteraction({ function: "vrf" }, { vrf: true });
console.log("txId: ", txId?.originalTxId);

const state = await contract.readState();
console.log(JSON.stringify(state.cachedValue.state, undefined, 2));

Both contractTxIds are actually valid. Their code being:

export async function handle(state, action) {
    const balances = state.balances;
    const input = action.input;
    const caller = action.caller;

    if (input.function === 'vrf') {
        if (!state.vrf) {
            state.vrf = {};
        }

        state.vrf[SmartWeave.transaction.id] = SmartWeave.vrf.value;

        return { state };
    }

    throw new Error("unrecognized input");
}

When running the example with Mainnet enabled the error is logged 2 times on readState(). And for some reason when setting internalWrites to true the error is logged 3 times on writeInteraction. When running the example with Testnet enabled instead, the error isn't logged except when internalWrites is set to true, then the error is displayed on writeInteraction.

ppedziwiatr commented 2 years ago

Thanks for the report, I'll take a look asap.

ppedziwiatr commented 2 years ago

Hmm, so after a quick analysis, it seems that the 2nd. and 3rd. interactions (kkEyEaVhB52w5txTkY86z24M26hXYXRAKhtPHZlSUzg and pc1hzdsB7nxDqXul-5D6Vkul81QqH-DKX9cDk8B_ysM - which are throwing this TypeError: Cannot read properties of null (reading 'bigint') error )

image

were not created with the { vrf: true } parameter.

Whenever this parameter is set in the writeInteraction, the interaction has additional tag added - the Request-Vrf tag. Whenever the sequencer gets a transaction with such tag, it knows that the Vrf data needs to be generated for this interaction. Eg. compare the 4th interaction https://sonar.warp.cc/#/app/interaction/WDZQrU6kYS5dvB0AgPlW5qmCMvqry3eMB02978ja9JA

with the 2nd. and 3rd. interaction: https://sonar.warp.cc/?#/app/interaction/kkEyEaVhB52w5txTkY86z24M26hXYXRAKhtPHZlSUzg https://sonar.warp.cc/?#/app/interaction/pc1hzdsB7nxDqXul-5D6Vkul81QqH-DKX9cDk8B_ysM

Keep in mind that the vrf data must be generated "per transaction" that requires vrf feature.

@asiaziola , btw., we should probably add the generated vrf data to the interaction view in Sonar - i.e. the interaction.vrf field - e.g.

 {
    "index": "zCSO2f0IGdZU5HhPRoIWC7xrOAKPWvuva0bKMRQ2YeQ",
    "proof": "nDVZDOKoQEMCLlxc6h93iSPxq9wIh6D8C_IP_x1lM28-Qei95RSRx5zj4y_EY-0PQUR-9ugr3GONepdgGeFj8QS2GLzdLEzR0-jJD9AMFYJkBuLbMWU7Zftg1I_dNdGbA0bnRxQBtqHnmgdvBhPwqEmQEfvmcATVp0qhCJMJYtuu",
    "bigint": "92336413530255378415861824142636333877904920323116062551236747077519536447972",
    "pubkey": "03027295998c12c584a11b3ffcb07f58d5392bd271f5185b1799fe775350dc75ce"
  }
noomly commented 2 years ago

Oops yeah that makes sense for these two interactions... Although, I just tried again by deploying fresh new contracts and it seems the issue is present when internalWrites is set to true: the error is displayed when calling writeInteraction even if vrf is set to true.

Btw I agree it could be a nice addition to the Sonar to see this kind of data!

ppedziwiatr commented 2 years ago

Ah yeah, in case of internal writes that's a whole other story...if you've read the internal writes docs that I've made recently you probably remember that the first step is to dry run the newly created interaction and gather information about internal calls...and it's this dry run that's causing the issue here (because and the timenof a dry run there's no vrf data generated yet).

I believe the same issue would occur if you try to create a non-internal write interaction with strict option set to true...

I'll need to think for a while hownto solve this...

czw., 29 wrz 2022, 18:32 użytkownik Eyal Chojnowski < @.***> napisał:

Oops yeah that makes sense for these two interactions... Although, I just tried again by deploying fresh new contracts and it seems the issue is present when internalWrites is set to true: the error is displayed when calling writeInteraction even if vrf is set to true.

Btw I agree it could be a nice addition to the Sonar to see this kind of data!

— Reply to this email directly, view it on GitHub https://github.com/warp-contracts/warp/issues/232#issuecomment-1262524276, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUDH5TKUDJHWYUGS622ZSNLWAXABLANCNFSM6AAAAAAQYTTZKQ . You are receiving this because you were assigned.Message ID: @.***>

noomly commented 2 years ago

Yeah, I realized that's what was with strict: true but I did forget that internal writes rely on dry runs.... Which now that I think of it, causes a much bigger issue than I thought. Because this means that, due to the way vrfs and internal writes are created, an internal write cannot happen conditionally to a vrf... It could be worked around at the contract level by requiring two separate interactions (1. generate the vrf and store it; 2. run the internal write on the stored vrf) but it's less than ideal. However I don't really see a better way to achieve this without fundamentally changing how Warp implements internal writes. Should I open a new issue about this?

ppedziwiatr commented 2 years ago

We can continue within this issue (I've just updated the title).

The options I see here:

  1. Drop the support for the VRF entirely. It seems that no one have been using this so far (apart from some tests/experiments), so the damage should be minimal.
  2. Mark that VRF is non-compatible with IW and writing interactions in a strict mode
  3. For the sake of IW and strict mode - generate a mock VRF data - probably in a similar way, as it is being done for the local environment.
noomly commented 2 years ago

.1. Please don't! I do see a lot of cases where VRF could be very useful. Except if there is another known and secure way to handle randomness in smartcontracts, VRF is currently the only solution to opening Arweave martcontracts to a lot of new use cases. .3. This would only work for IW that are not triggered conditionally to a VRF, otherwise a mock VRF couldn't predict accurately whether this IW would be triggered

So yeah, I only see point 2 working right now, coupled with the workaround I mentioned in my previous comment...

ppedziwiatr commented 2 years ago

Decision - we'll go with the point 3. - i.e.:

For the sake of IW and strict mode - generate a mock VRF data - probably in a similar way, as it is being done for the local environment.

+ proper documentation with warns re. specific use cases (e.g. when the IW call would be dependent on the generated VRF value)

ppedziwiatr commented 2 years ago

hey @noomly , please give 1.2.13 version a try.

ppedziwiatr commented 2 years ago

ofc. I've forgot about documenting this.. 🤡

asiaziola commented 2 years ago

@noomly @ppedziwiatr vrf has been added to interaction view on sonar

ppedziwiatr commented 2 years ago

@noomly @ppedziwiatr vrf has been added to interaction view on sonar

image

seems to work...

noomly commented 2 years ago

Amazing, many thanks to you both, on behalf of everyone that'll be able to enjoy this feature in the future! :smile: I'll give a try to 1.2.13 tomorrow.