Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: specify encoding when calling bytes [APE-1186] #1539

Merged
merged 5 commits into from
Jul 24, 2023
Merged

Conversation

z80dev
Copy link
Contributor

@z80dev z80dev commented Jul 13, 2023

What I did

Specify encoding as utf-8 when calling bytes

fixes: #1535

fixes: APE-1181

How I did it

in this case we should always be dealing with utf8 strings

How to verify it

I kept getting contract deployment failures like reported in the discord and in #1535
with this fix I was able to deploy contracts successfully

Checklist

  • All changes are completed
  • New test cases have been added
  • Documentation has been updated

@vany365 vany365 changed the title specify encoding when calling bytes specify encoding when calling bytes [APE-1186] Jul 13, 2023
@z80dev z80dev changed the title specify encoding when calling bytes [APE-1186] fix: specify encoding when calling bytes [APE-1186] Jul 13, 2023
Copy link
Member

@antazoey antazoey left a comment

Choose a reason for hiding this comment

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

test?

antazoey
antazoey previously approved these changes Jul 17, 2023
Copy link
Member

@antazoey antazoey left a comment

Choose a reason for hiding this comment

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

test can be really simple:

def test_txn_str_when_data_is_bytes(ethereum):
    txn = ethereum.create_transaction(data=HexBytes("0x123"))
    actual = str(txn)
    assert isinstance(actual, str)

^ this fails on main

@victor-ego
Copy link

I recently encountered an analogous issue, and the resolution was accomplished via the installation of this specific branch.

This is to shed light on a plausible implication associated with the constant handling of UTF-8 formatted strings, particularly when they are passed as ape strings that represent parameters such as the Universal Resource Identifier (URI) for NFT images.

In the wake of recent updates to the Vyper, Python, and Ape environments, I have started experiencing difficulties with a previously operational contract. This has specifically impacted the ability to load NFT image assets on various marketplaces including, but not limited to, Rarible and OpenSea.

Token Minted Transaction: Goerli Etherscan
Base URI: ipfs://QmVTXG1tr3QpgwQ3n67qYmPqHgVbSMpDQnNAyqFtLD2HPH
Contract Source: GitHub - Ape Academy ERC721

Please note the associated links for further perusal and analysis.

Copy link
Member

@antazoey antazoey left a comment

Choose a reason for hiding this comment

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

LGTM - I added the test for you.

@antazoey antazoey enabled auto-merge (squash) July 24, 2023 14:13
@antazoey antazoey merged commit d477765 into ApeWorX:main Jul 24, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deployment contract encountered an error [APE-1181]
4 participants