Skip to content

Conversation

yann300
Copy link
Contributor

@yann300 yann300 commented Jul 27, 2021

ask the user to input maxFee and maxPriorityFee if the chain is eip 1559 enabled.
references for the implementation:
https://hackmd.io/4YVYKxxvRZGDto7aq7rVkg?view
https://github.com/ethereum/eth1.0-specs/blob/master/network-upgrades/ecosystem-readiness.md

EDIT: the last set of changes updates remix-ide to it default to London when the app starts
EDIT: this commit 533d7f9 use berlin roll back unit testing to use berlin, fix for making sure tests run with london will come later

@yann300 yann300 force-pushed the london-support branch 4 times, most recently from e4a73ae to 4944a39 Compare July 27, 2021 15:33
el.querySelector('#gasprice').value = gasPriceValue
onGasPriceChange()
}
if (el.querySelector('#maxfee') && network && network.lastBlock && network.lastBlock.baseFeePerGas && el.querySelector('#maxfee')) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

code duplication

const block = await web3.eth.getBlock('latest')
// we can't use the blockGasLimit cause the next blocks could have a lower limit : https://github.com/ethereum/remix/issues/506
this.blockGasLimit = (block && block.gasLimit) ? Math.floor(block.gasLimit - (5 * block.gasLimit) / 1024) : this.blockGasLimitDefault
this.lastBlock = block
Copy link
Contributor Author

Choose a reason for hiding this comment

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

_updateChainContext should be updated as soon as possible when a new provider is switched to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"@ethereumjs/vm": "^5.4.1",
"@ethereumjs/block": "^3.4.0",
"@ethereumjs/tx": "^3.3.0",
"@ethereumjs/vm": "^5.5.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

might need to check what to do with maxFee and maxPriorityFee.

Copy link
Collaborator

@Aniket-Engg Aniket-Engg left a comment

Choose a reason for hiding this comment

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

I think JavaScript VM (London) should be first option in Environment dropdown in Deploy and Run

<div>
<div>You are about to create a transaction on the Main Network. Confirm the details to send the info to your provider.
<br>The provider for many users is MetaMask. The provider will ask you to sign the transaction before it is sent to the Main Network.</div>
<div>You are about to create a transaction on ${network.name}. Confirm the details to send the info to your provider.
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe "...on ${network.name} network. ..." would be the correct info

<div>You are about to create a transaction on the Main Network. Confirm the details to send the info to your provider.
<br>The provider for many users is MetaMask. The provider will ask you to sign the transaction before it is sent to the Main Network.</div>
<div>You are about to create a transaction on ${network.name}. Confirm the details to send the info to your provider.
<br>The provider for many users is MetaMask. The provider will ask you to sign the transaction before it is sent to ${network.name}.</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

@LianaHus
Copy link
Collaborator

LianaHus commented Aug 2, 2021

Some notes about UI changes after a discussion/call (@Aniket-Engg @LianaHus)

  • Code goes before all gas calculations.
  • Max tx fee goes the bottom and if possible should be highlighted to catch the attention as the most important info.
  • Error about minimum fee should be either orange or red or should be replaced with validation(should be decided later)
    In any case, it will disable the 'Confirm' button(?)

@Aniket-Engg Aniket-Engg merged commit 8a8ad19 into master Aug 3, 2021
@Aniket-Engg Aniket-Engg deleted the london-support branch August 3, 2021 07:25
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.

3 participants