Skip to content
This repository was archived by the owner on Nov 10, 2023. It is now read-only.

Show ETH value input for payable methods (fix for solidity v0.6x)#1717

Merged
dasanra merged 3 commits intodevelopmentfrom
issue-1680
Dec 14, 2020
Merged

Show ETH value input for payable methods (fix for solidity v0.6x)#1717
dasanra merged 3 commits intodevelopmentfrom
issue-1680

Conversation

@nicosampler
Copy link
Contributor

Closes partially #1680

@nicosampler nicosampler self-assigned this Dec 11, 2020
@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link

github-actions bot commented Dec 11, 2020

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@ghost
Copy link

ghost commented Dec 11, 2020

Travis automatic deployment:
https://pr1717--safereact.review.gnosisdev.com/rinkeby/app

@ghost
Copy link

ghost commented Dec 11, 2020

Travis automatic deployment:
https://pr1717--safereact.review.gnosisdev.com/volta/app

@ghost
Copy link

ghost commented Dec 11, 2020

Travis automatic deployment:
https://pr1717--safereact.review.gnosisdev.com/rinkeby/app

@ghost
Copy link

ghost commented Dec 11, 2020

Travis automatic deployment:
https://pr1717--safereact.review.gnosisdev.com/volta/app

@ghost
Copy link

ghost commented Dec 14, 2020

Travis automatic deployment:
https://pr1717--safereact.review.gnosisdev.com/rinkeby/app

@ghost
Copy link

ghost commented Dec 14, 2020

Travis automatic deployment:
https://pr1717--safereact.review.gnosisdev.com/volta/app

@dasanra dasanra merged commit 89c7482 into development Dec 14, 2020
@dasanra dasanra deleted the issue-1680 branch December 14, 2020 10:01
@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2020
@francovenica
Copy link
Contributor

francovenica commented Dec 14, 2020

I tried using the ABI that Tobi gave as example and the method "Deposit" showed the ETH field

[{"inputs":[],"stateMutability":"nonpayable","type":"constructor"},{"anonymous":false,"inputs":[{"indexed":false,"internalType":"bytes","name":"pubkey","type":"bytes"},{"indexed":false,"internalType":"bytes","name":"withdrawal_credentials","type":"bytes"},{"indexed":false,"internalType":"bytes","name":"amount","type":"bytes"},{"indexed":false,"internalType":"bytes","name":"signature","type":"bytes"},{"indexed":false,"internalType":"bytes","name":"index","type":"bytes"}],"name":"DepositEvent","type":"event"},{"inputs":[{"internalType":"bytes","name":"pubkey","type":"bytes"},{"internalType":"bytes","name":"withdrawal_credentials","type":"bytes"},{"internalType":"bytes","name":"signature","type":"bytes"},{"internalType":"bytes32","name":"deposit_data_root","type":"bytes32"}],"name":"deposit","outputs":[],"stateMutability":"payable","type":"function"},{"inputs":[],"name":"get_deposit_count","outputs":[{"internalType":"bytes","name":"","type":"bytes"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"get_deposit_root","outputs":[{"internalType":"bytes32","name":"","type":"bytes32"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"bytes4","name":"interfaceId","type":"bytes4"}],"name":"supportsInterface","outputs":[{"internalType":"bool","name":"","type":"bool"}],"stateMutability":"pure","type":"function"}]

Also used a contract compiled with an older version of solidity, to make sure that old contracts still work with this fix
https://rinkeby.etherscan.io/address/0x3d7c4042365d04c98f58906966ca4c816acf592a#code
The method "Vote" also has a field with ETH that should display, and it does

I test the same scenarios in the new tx builder with the fix and it worked fine as well
safe-global/safe-react-apps#84
https://ipfs.io/ipfs/QmYES1Se6i6679z3PfQ62bydgVVEoSRUabvjB35DfUGPGA/

Looks good to me

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.

5 participants