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

Fix CI #1172

Merged
merged 4 commits into from
Jun 17, 2020
Merged

Fix CI #1172

merged 4 commits into from
Jun 17, 2020

Conversation

bingen
Copy link
Contributor

@bingen bingen commented Jun 17, 2020

No description provided.

For Vault, Agent and Finance. To fix Istanbul related issues.
See https://github.com/aragon/aragonOS/releases/tag/v4.3.0 for more context.
ßingen added 2 commits June 17, 2020 15:02
`isValidSignature` was calling the `bytes` version due to overloading
issue with Truffle.
@bingen bingen marked this pull request as ready for review June 17, 2020 13:04
@bingen bingen requested a review from sohkai June 17, 2020 13:04
@bingen bingen changed the title Fix ci Fix CI Jun 17, 2020
@auto-assign auto-assign bot requested review from bpierre and Evalir June 17, 2020 13:05
Copy link
Contributor

@sohkai sohkai left a comment

Choose a reason for hiding this comment

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

😍 This is so 🔥, thanks ❤️ @bingen!



contract AgentMock is Agent {
function isValidSignatureBytes32(bytes32 _hash, bytes memory _signature) public view returns (bytes4) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we leave a comment on why this is necessary?

})
})

context('when there are two allowed token', function () {
it('expends ~49k gas in setting a allocation tokens per token', async () => {
const gas1 = 49
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a huge fan of these variable names (if we even need them at all)

Copy link
Contributor

@sohkai sohkai left a comment

Choose a reason for hiding this comment

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

LGTM! 🙏

@bingen bingen merged commit ea97425 into master Jun 17, 2020
@bingen bingen deleted the fix_ci branch June 17, 2020 20:20
ramilexe pushed a commit to ConsiderItDone/aragon-apps that referenced this pull request Dec 9, 2021
* Upgrade aragonOS dependency

For Vault, Agent and Finance. To fix Istanbul related issues.
See https://github.com/aragon/aragonOS/releases/tag/v4.3.0 for more context.

* payroll: fix gas values in tests

* agent: fix tests

`isValidSignature` was calling the `bytes` version due to overloading
issue with Truffle.

* Fix CI, address PR aragon#1172 comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants