Skip to content

Make contracts 4.24 compatible#951

Merged
shrugs merged 2 commits into
OpenZeppelin:masterfrom
artiebits:feature/4.24-compatible-contracts
Jun 13, 2018
Merged

Make contracts 4.24 compatible#951
shrugs merged 2 commits into
OpenZeppelin:masterfrom
artiebits:feature/4.24-compatible-contracts

Conversation

@artiebits

Copy link
Copy Markdown
Contributor

🚀 Description

  • 📘 I've reviewed the OpenZeppelin Contributor Guidelines
  • ✅ I've added tests where applicable to test my new functionality.
  • 📖 I've made sure that my contracts are well-documented.
  • 🎨 I've run the JS/Solidity linters and fixed any issues (npm run lint:all:fix).

Truffle updated solc to 0.4.24 as well.

Warnings fixed:

  • keccak256 only accepts a single "bytes" argument.
  • Using contract member "balance" inherited from the address type is deprecated, use "address(contract).balance" instead.

@come-maiz come-maiz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Awesome @artiebits, thanks a lot. We usually wait around a month after solc is released to update, so this will be perfect for our next release.
However, because we were slow to review this now it has some conflicts. Can you please rebase?
Also, please update the truffle version.

Comment thread package.json Outdated
"solidity-coverage": "^0.5.0",
"solium": "^1.1.7",
"truffle": "^4.1.8",
"truffle": "^4.1.9",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@artiebits

Copy link
Copy Markdown
Contributor Author

@ElOpio np, rebased and updated

@come-maiz

Copy link
Copy Markdown

Sorry @artiebits, once more.
@shrugs @frangio can we land this next, so there are no more conflicts on the way?

@shrugs

shrugs commented Jun 12, 2018

Copy link
Copy Markdown
Contributor

oops we did it again 😓

I'll personally avoid merging any more PRs until this one is merged. sorry for the extra work, @artiebits

@axic

axic commented Jun 12, 2018

Copy link
Copy Markdown
Contributor

Any chance merging this and #1002? It would simplify our life at the Solidity repo a lot :)

@shrugs

shrugs commented Jun 12, 2018

Copy link
Copy Markdown
Contributor

@axic will do!

@shrugs

shrugs commented Jun 12, 2018

Copy link
Copy Markdown
Contributor

Also it'd be cool if these sort of upgrades were done automatically by a bot in the future, since it's a pretty easily automatable (but human-checkable) operation

@axic

axic commented Jun 12, 2018

Copy link
Copy Markdown
Contributor

Btw, people with write access to this repo should be able (unless specifically disabled by the owner) to push to @artiebits PRs.

@artiebits

Copy link
Copy Markdown
Contributor Author

no worries, updated

@frangio

frangio commented Jun 13, 2018

Copy link
Copy Markdown
Contributor

it'd be cool if these sort of upgrades were done automatically by a bot in the future

This should be possible using Solium's fix mode!

shrugs
shrugs previously approved these changes Jun 13, 2018

@shrugs shrugs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a quick search for 0.4.23 in the project shows no missing pragmas, so we're good to go

@shrugs shrugs dismissed their stale review June 13, 2018 18:16

missing abi.encodePacked

@shrugs shrugs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@shrugs shrugs merged commit 5daaf60 into OpenZeppelin:master Jun 13, 2018
@frangio frangio mentioned this pull request Jul 23, 2018
2 tasks
@Semaerdem3536

Copy link
Copy Markdown

🚀 Description

  • 📘 I've reviewed the OpenZeppelin Contributor Guidelines
  • ✅ I've added tests where applicable to test my new functionality.
  • 📖 I've made sure that my contracts are well-documented.
  • 🎨 I've run the JS/Solidity linters and fixed any issues (npm run lint:all:fix).

Truffle updated solc to 0.4.24 as well.

Warnings fixed:

  • keccak256 only accepts a single "bytes" argument.
  • Using contract member "balance" inherited from the address type is deprecated, use "address(contract).balance" instead.

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.

6 participants