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

feat: possibility to set metadata for erc20 contracts #837

Merged
merged 3 commits into from
Sep 14, 2023

Conversation

aleksuss
Copy link
Member

Description

The PR adds functions for setting (set_erc20_metadata) and getting (get_erc20_metadata) metadata of ERC-20 contracts deployed with deploy_erc20_token transaction.

Performance / NEAR gas cost considerations

There are no performance changes.

Testing

The corresponding test has been added.

Additional information

Probably, there should be a new role for set_erc20_metadata transaction. I guess it should be added in a new PR.

@aleksuss aleksuss force-pushed the feat/aleksuss/set_erc20_metadata branch from 8321d0f to ab6fa78 Compare September 12, 2023 16:05
Copy link
Member

@birchmd birchmd left a comment

Choose a reason for hiding this comment

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

Nice work! Just a few comments.

engine/src/contract_methods/connector.rs Outdated Show resolved Hide resolved
engine-types/src/parameters/connector.rs Outdated Show resolved Hide resolved
@aleksuss aleksuss force-pushed the feat/aleksuss/set_erc20_metadata branch from be5a5b6 to 61dbd5a Compare September 13, 2023 09:13
@aleksuss
Copy link
Member Author

One bad moment of changing borsh to JSON is that it increases the number of functions and size of the smart contract.
cc @birchmd

@birchmd
Copy link
Member

birchmd commented Sep 13, 2023

Increasing the number of functions is ok in my opinion. The reason for counting them is because there is a hard limit on the number of functions Near allows in a contract. But this limit is 10000, so we are not close to exceeding it.

How much bigger is the contract size? Will it be offset by the wasm opt improvment?

@aleksuss
Copy link
Member Author

How much bigger is the contract size? Will it be offset by the wasm opt improvement?

1166962 vs 1060706 (develop), diff: 106256 bytes. After applying wasm-opt the size becomes 1041185 bytes.

Copy link
Member

@birchmd birchmd 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 think the increase in contract size is worth the utility these methods provide.

engine/src/contract_methods/connector.rs Show resolved Hide resolved
@aleksuss aleksuss added this pull request to the merge queue Sep 14, 2023
Merged via the queue into develop with commit 0731841 Sep 14, 2023
20 checks passed
@aleksuss aleksuss deleted the feat/aleksuss/set_erc20_metadata branch September 14, 2023 10:40
aleksuss added a commit that referenced this pull request Sep 25, 2023
## Description

The PR adds functions for setting (`set_erc20_metadata`) and getting
(`get_erc20_metadata`) metadata of ERC-20 contracts deployed with
`deploy_erc20_token` transaction.

## Performance / NEAR gas cost considerations

There are no performance changes.

## Testing

The corresponding test has been added.

## Additional information

Probably, there should be a new role for `set_erc20_metadata`
transaction. I guess it should be added in a new PR.
@aleksuss aleksuss mentioned this pull request Sep 25, 2023
aleksuss added a commit that referenced this pull request Sep 25, 2023
- Added the possibility to use native NEAR instead of wNEAR on Aurora by
[@karim-en]. ([#750])

- Added hashchain integration by [@birchmd]. ([#831])

- Added functions for setting and getting metadata of ERC-20 contracts
deployed with `deploy_erc20_token` transaction
  by [@aleksuss]. ([#837])

---------

Co-authored-by: Michael Birch <michael.birch@aurora.dev>
Co-authored-by: Evgeny Ukhanov <evgeny@aurora.dev>
Co-authored-by: Karim <karim@aurora.dev>
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.

None yet

5 participants