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

Invoke contracts #151

Merged
merged 7 commits into from
Mar 14, 2024
Merged

Invoke contracts #151

merged 7 commits into from
Mar 14, 2024

Conversation

ftheirs
Copy link
Contributor

@ftheirs ftheirs commented Feb 29, 2024

Improve display of params when EVM Invoke call is ERC20 transfer
Explicit check that users cannot make blindsign with normal mode
Bugfix ERC20 transfer
Improve format from amounts (token + decimals for several fields)
Update dependencies

🔗 zboto Link

Copy link
Member

@emmanuelm41 emmanuelm41 left a comment

Choose a reason for hiding this comment

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

Left a few comments!

app/src/eth_erc20.h Outdated Show resolved Hide resolved
app/src/parser_impl.c Show resolved Hide resolved
tests/testvectors/invoke_contracts.json Outdated Show resolved Hide resolved
return parser_ok;
}

parser_error_t printInvokeEVM(const fil_base_tx_t *txObj,

Check warning

Code scanning / CodeQL

Poorly documented large function Warning

Poorly documented function: fewer than 2% comments for a function of 133 lines.
Copy link
Member

@emmanuelm41 emmanuelm41 left a comment

Choose a reason for hiding this comment

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

Just a few minor comments. I have to be honest. It is not easy to review hahah. Something I noticed from zemu images, and I am not sure is are we saying "Transfer" when we are clear signing a well-known contract? I didn't see it on the images.

app/Makefile.version Outdated Show resolved Hide resolved
app/src/eth_erc20.c Show resolved Hide resolved
app/src/parser_invoke_evm.c Outdated Show resolved Hide resolved
app/src/parser_invoke_evm.c Outdated Show resolved Hide resolved
@ftheirs ftheirs force-pushed the invoke_contracts branch 4 times, most recently from 1e2eda0 to fa8268e Compare March 7, 2024 21:14
@ftheirs ftheirs merged commit eb08475 into main Mar 14, 2024
37 checks passed
@ftheirs ftheirs deleted the invoke_contracts branch March 14, 2024 14:10
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.

2 participants