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

feat: upgrade to solc 0.6.10 and added GsnCapableDocumentStore #76

Merged
merged 10 commits into from
Nov 27, 2020

Conversation

justussoh
Copy link
Contributor

This PR includes:

  • upgrading of contracts to solc 0.6.10
  • adding of Gsn Capable Document Store
  • added Naive Paymaster to pay for the relayed transaction

Removed:

  • Upgradable factory proxy (deprecated)

Todo:

@@ -2,6 +2,9 @@
"extends": ["solhint:recommended"],
"plugins": ["prettier"],
"rules": {
"indent": ["error", 2]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@yehjxraymond yehjxraymond left a comment

Choose a reason for hiding this comment

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

I don't have a good understanding of how GSN works yet, just a very brief idea. I was looking out for a test that can show that one party can fund another's transaction but dont find the test in there.

import "@opengsn/gsn/contracts/BaseRelayRecipient.sol";
import "@opengsn/gsn/contracts/interfaces/IKnowForwarderAddress.sol";

contract GsnCapableDocumentStore is BaseRelayRecipient, IKnowForwarderAddress, GsnCapable {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not extend the base document store too?

test/GsnCapableDocumentStore.js Show resolved Hide resolved
test/GsnCapableDocumentStore.js Show resolved Hide resolved

const issue = documentMerkleRoot => instance.issue(documentMerkleRoot);

describe("constructor", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

if inheriting from document store base contract can remove these test?

test/NaivePaymaster.js Show resolved Hide resolved
.gitignore Show resolved Hide resolved
justussoh and others added 2 commits November 25, 2020 12:20
* fix: bytecode

* fix: refactored data

* fix: lint

Co-authored-by: Raymond Yeh <[email protected]>
Copy link
Contributor

@yehjxraymond yehjxraymond left a comment

Choose a reason for hiding this comment

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

lgtm.
can help to tidy up the dependent code in another PR?

@justussoh justussoh merged commit f000ed5 into master Nov 27, 2020
@justussoh justussoh deleted the feat/sol-0.6.10 branch November 27, 2020 09:44
@john-dot-oa
Copy link

🎉 This PR is included in version 2.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants