-
Notifications
You must be signed in to change notification settings - Fork 336
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 return types in asset erc20 and refactor asseterc20precompileset #990
fix return types in asset erc20 and refactor asseterc20precompileset #990
Conversation
…ss AssetId in PrecompileSet
…rn-types-in-asset-erc20-and-refactor-asseterc20precompileset
@@ -194,7 +194,7 @@ fn export_current_state() { | |||
|
|||
// Let it produce some blocks. | |||
// This fails if is not a minimum of 25 | |||
thread::sleep(Duration::from_secs(25)); | |||
thread::sleep(Duration::from_secs(35)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess because of the JIT compilation issue. but the CI was complaining:(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can [hopefully] be reverted once 993 is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed
…990) * Fix return type matching the open zeppeling ERC20 and the way to access AssetId in PrecompileSet * Start typescrypt testing this * Fix typescript integration tests * Check if caller is same as from * add delgeate calls to contract instance * add more cases considering delegate and normal calls * Add contract compiled * Increment node wake up time yet again * Increment the proper timeout * Add PR suggestions
* fix return types in asset erc20 and refactor asseterc20precompileset (#990) * Fix return type matching the open zeppeling ERC20 and the way to access AssetId in PrecompileSet * Start typescrypt testing this * Fix typescript integration tests * Check if caller is same as from * add delgeate calls to contract instance * add more cases considering delegate and normal calls * Add contract compiled * Increment node wake up time yet again * Increment the proper timeout * Add PR suggestions * Bump specs to 902 in moonbase
What does it do?
It solves some issues found in assets-erc20-precompiles when being called form a smart contract. IN particular:
DelegateCalls where not working, since we were fetching the assetId from execution_address, but delegateCalls take the calling smart contract as execution_address. I made a refactor to avoid this situation, and I think the code looks cleaner now.
Calls from a SC to transfer, approve, TransferFrom were not working, as we were not returning a boolean as the execution result. This is required by the ERC-20 standard, so I added this fixing the corresponding tests.
Added a ton of typescript tests regarding delegateCalls and smart contract calls cases. I think I have covered most of the stuff now
What important points reviewers should know?
Is there something left for follow-up PRs?
What alternative implementations were considered?
Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?
What value does it bring to the blockchain users?