Skip to content
This repository has been archived by the owner on Sep 5, 2023. It is now read-only.

bytes32(string) fix #900

Merged
merged 9 commits into from
Feb 9, 2023
Merged

bytes32(string) fix #900

merged 9 commits into from
Feb 9, 2023

Conversation

rjnrohit
Copy link
Contributor

@rjnrohit rjnrohit commented Feb 6, 2023

closes #892

@swapnilraj
Copy link
Contributor

the tests pass so it looks good but could explain the bug and how your change fixes it?

@rjnrohit
Copy link
Contributor Author

rjnrohit commented Feb 7, 2023

the tests pass so it looks good but could explain the bug and how your change fixes it?

This fixes the bug because previously arg.kind remains as a string all the way upto cairo writer, And in case of strings to fixedBytes conversion, the cairoWriter writes the expression as a single felt value using node.value despite node having typestring uint256 . Since, all FixedBytes conversions are later converted into number/hexNumbers, the, arg.kind for string conversion shouldn't be stay as string

switch (node.kind) {
      case LiteralKind.Number:
        switch (primitiveTypeToCairo(node.typeString)) {
          case 'Uint256': {
            const [high, low] = divmod(BigInt(node.value), BigInt(Math.pow(2, 128)));
            return [`Uint256(low=${low}, high=${high})`];
          }
          case 'felt':
            return [node.value];
          default:
            throw new TranspileFailedError('Attempted to write unexpected cairo type');
        }
      case LiteralKind.Bool:
        return [node.value === 'true' ? '1' : '0'];
      case LiteralKind.String:
      case LiteralKind.UnicodeString: {
        if (
          node.value.length === node.hexValue.length / 2 &&
          node.value.length < 32 &&
          node.value.split('').every((v) => v.charCodeAt(0) < 127)
        ) {
          return [`'${node.value}'`];
        }
        return [`0x${node.hexValue}`];
      }

@swapnilraj
Copy link
Contributor

Nice catch and good explanation! thanks

@swapnilraj swapnilraj merged commit bb927ff into develop Feb 9, 2023
@swapnilraj swapnilraj deleted the bytes32 branch February 9, 2023 08:58
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.

bytes32 in BuiltinHandler pass
3 participants