wax-office-of-inspector-general / wax-developer

WAX Developer Portal - Learn, Build, Operate & Create
https://developer.wax.io
10 stars 52 forks source link

'RNG Basics" issues #83

Closed NKCSS closed 1 year ago

NKCSS commented 1 year ago

In rng_basics.md they describe that you can use the transaction hash to generate a signing_value inside your contract.

The big issue is, this generates a 256-bit value, which isn't the last parameter that is passed to the function, when the function expects a 64-bit integer followed by the caller.

I figured I'd see if there was some magic I wasn't accounting for, but no, when you actually do this, it will take the bits after the first 64-bits at the caller value, resulting in a random caller and this failing the auth check in the orng.wax contract.

image

You could do something like this (I tested & verified it), but that would not guarantee 'no collissions' as advertised in the article.

uint64_t uint64_t_from_txhash(){
  auto size = transaction_size();
  char buf[size];
  auto read = read_transaction(buf, size);
  check(size == read, "read_transaction() has failed.");
  auto tx_signing_value = sha256(buf, size);
  auto tx_raw_signing_value = tx_signing_value.extract_as_byte_array().data();
  uint64_t result;
  memcpy(&result, &tx_raw_signing_value, sizeof(uint64_t));
  return result;
}

Also, minor note; if you introduce stuff that require imports, please add them to the article/have a lookup somewhere (sha256 requires an import of <eosio/crypto.hpp>

NKCSS commented 1 year ago

update: the first 64-bits of the tx apparently are not the part that have the block number, etc 😅so as soon as I try to submit a tx with similar data, it returns a duplicate number, so I'll have to see what the tx structure is to at least get the part that does change with every execution 😅

update2: had a bug in the code; updated the snippet and it now works (ran a bunch of tests).

3dkrender commented 1 year ago

Indeed, the use of a 256-bit hash is not the best example. More information is missing. I have interacted a lot with this smart contract. I will try to improve the content of this tutorial in the coming days.

includenull commented 1 year ago

I have always used this without any issues, its the same method the atomicpacks contract uses.

uint64_t generate_signing_value() {
  size_t size = transaction_size();
  char buf[size];
  uint32_t read = read_transaction(buf, size);
  check(size == read, string("read_transaction() has failed.").c_str());
  checksum256 tx_id = sha256(buf, read);
  uint64_t signing_value;
  memcpy(&signing_value, &tx_id, sizeof(signing_value));
  return signing_value;
}

Although the atomicpacks contract also has an extra step of checking if the signing_value was already used which should not be done, its not intended use and the table its checking might go away soon.

rakeden commented 1 year ago

PR for updated content has been merged: https://developer.wax.io/en/tutorials/create-wax-rng-smart-contract/rng_basics

rakeden commented 1 year ago

code sample changed in #85

includenull commented 1 year ago

I still don't think advising to check the signvals table is a good idea.

This is part of the old RNG system and could be removed in future updates.

3dkrender commented 1 year ago

Yep. This is only an example of use. Obviously in the future anything can change due to future updates. When that happens it will be necessary to revise the examples.

I didn't want to introduce more actors but, if someone asks me, I would recommend using an NFT ID as signing_value instead of trying to generate a unique and valid one.

I would also recommend developing a customised and complete system, including Oracle. It's been proven that relying on external services can leave your application hanging.

But this is all at another level and this tutorial is at a basic level.

rakeden commented 1 year ago

closing this issue