Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Send value & contract execute gas limit warnings #3512

Merged
merged 5 commits into from
Nov 22, 2016

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Nov 18, 2016

Show gas block limit exceeded & transaction execution failures warnings.

Closes https://github.com/ethcore/parity/issues/3366

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Nov 18, 2016
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 8d339ad on jg-transact-gas-warnings into * on master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f827ade on jg-transact-gas-warnings into * on master*.

// You should have received a copy of the GNU General Public License
// along with Parity. If not, see <http://www.gnu.org/licenses/>.

const DEFAULT_GAS = '21000';
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these values always fixed, for any network?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For a account -> account transfer, it is always 21k. (We never directly use this value, any gas passed is always from estimations, so even when we know it is 21k, we still call estimateGas and use that value. Worst-case it is a placeholder until the first call to estimateGas)

@ngotchac ngotchac added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Nov 22, 2016
@gavofyork gavofyork merged commit a722bac into master Nov 22, 2016
@gavofyork gavofyork deleted the jg-transact-gas-warnings branch November 22, 2016 16:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants