Skip to content
This repository was archived by the owner on Jul 12, 2023. It is now read-only.

Conversation

@pixelcircuits
Copy link
Contributor

Moved helper functions from the predicate into their own utils library and added a new helper function tx_script_bytecode_hash to easily get an unpadded script hash.

Note: there is a future issue to tweak this new function to use GTF in the future

@pixelcircuits pixelcircuits linked an issue Aug 31, 2022 that may be closed by this pull request
Copy link
Contributor

@Braqzen Braqzen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. main.sw
    • Has unused imports
      • ContractId
      • INPUT_MESSAGE
    • MIN_GAS const can probably be moved into the manifest file. The latest forc allows us to load values from the file as a pseudo constructor. I haven't used this myself yet
  2. utils.sw
    • Has unused imports
      • OUTPUT_CHANGE
      • OUTPUT_CONTRACT
      • OUTPUT_VARIABLE
      • tx_gas_limit
      • tx_inputs_count
      • tx_output_type
      • tx_outputs_count
      • sha256
    • Similar comment regarding moving constants into manifest
    • I would keep all the imports together and then below them declare the constants (if not put in manifest) rather than have some imports at the top and then a core::num and constants somewhere at the bottom of the file. Keeping things neatly together, at the top, makes it easier to navigate when searching

@adlerjohn
Copy link

If there isn't an issue in the Sway repo to warn on unused imports then please file one.

Copy link

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New versions of forc and fuels-rs are around the corner. Would it be better to wait a few days and do this properly with the new functions instead of hard-coding things that will have to get changed later?

// This ensures the coins can only be spent in a call to `TokenContract.finalizeDeposit()`
// Note: The script must be right-padded to the next full word before hashing, to match with `get_script_bytecode()`
const SPENDING_SCRIPT_HASH = 0x2d235589506d17993e0b7aca4407a5ac1c325efd9d704ff94696a8f7c012ab9d;
const SPENDING_SCRIPT_HASH = 0x6856bb03b84d876ee60c890be5b0602d0f7480d375917a660da3115e8e008ddb;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like the perfect usecase for configuration-time constants!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be deferred to a subsequent PR.

@pixelcircuits
Copy link
Contributor Author

pixelcircuits commented Sep 2, 2022

If there isn't an issue in the Sway repo to warn on unused imports then please file one.

Someone already made an issue for it: FuelLabs/sway#1298

@pixelcircuits
Copy link
Contributor Author

New versions of forc and fuels-rs are around the corner. Would it be better to wait a few days and do this properly with the new functions instead of hard-coding things that will have to get changed later?

Yeah, I think I'm gonna scrap this PR since I've started working with using the new GTF refactor. Merging this minor issue may not be worth the effort

@pixelcircuits
Copy link
Contributor Author

Closing to prioritize new sway and sdk releases

@adlerjohn adlerjohn deleted the 7-implement-simpler-hash-function branch September 2, 2022 17:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement simpler hash function

3 participants