Solidity: Introduce static elementary types to use

[chriseth]

Code like IntegerType(160, IntegerType::Modifier::Address); is very common and should be replaced by something like ElementaryTypes::Address (i.e. ElementaryTypes should be a struct that contains several static members initialized to specific instances of the types) at least for the more common elementary types to improve readability.

Note that the Type class inherits from std::enabled_shared_from_this and thus is always a shared_ptr. The elements of the ElementaryTypes struct should be the types themselves, but this has to be checked during implementation what is more common.

The elementary types should be:

bytes32, byte, address, uint256 and others that are found in the code.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__This issue now has a funding of 225.0 DAI (225.0 USD @ $1.0/DAI) attached to it.__

• If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $21,025.18 more funded OSS Work available on the Gitcoin Issue Explorer

[vaibhavchellani]

@chriseth can you please point me to places where i would need to make changes and add the struct containing Elementary types so that i can accelerate the work a bit , also i could not find any instances of bytes32 , unit256 or bytes being used like IntegerType(160, IntegerType::Modifier::Address);

[chriseth]

@vaibhavchellani please start with all occurrences of IntegerType(160, IntegerType::Modifier::Address). I think something like ArrayType(DataLocation::Memory) and ArrayType(DataLocation::Memory, true) should also be replaced by something like ElementaryTypes::BytesMemory and ElementaryTypes::StringMemory, respectively.

The struct should be placed in Types.h, I think.

[gitcoinbot]

@vaibhavchellani Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[vaibhavchellani]

yeah i am working on this issue

[vs77bb]

Thanks for the update @vaibhavchellani!

[gitcoinbot]

@vaibhavchellani Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[vaibhavchellani]

yeah i am trying to wrap my head around the codebase , i actually did a basic implementation and shall create a PR before the weekend , sorry for the delay guys !

[gitcoinbot]

@vaibhavchellani Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[vs77bb]

Hi @vaibhavchellani do you have an update to provide today/tomorrow? If not, we may have to send it back to the crowd.

[vaibhavchellani]

hey @vs77bb , opening up a PR before tomorrow .

[gitcoinbot]

@vaibhavchellani Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[vaibhavchellani]

yes i am working on it , facing some issues

[vs77bb]

Hi @vaibhavchellani after checking with @chriseth we decided it's time to return this to the crowd. Thanks for your efforts and please do check Gitcoin again for other potential issues to work!

[lastmjs]

I've replaced all occurrences of IntegerType(160, IntegerType::Modifier::Address) with ElementaryTypes::Address, ArrayType(DataLocation::Memory) with ElementaryTypes::BytesMemory, and ArrayType(DataLocation::Memory, true) with ElementaryTypes::StringMemory. Compilation completes successfully. Any other types that should be replaced?

[lastmjs]

Also, is there anything in the README / documentation that I need to add? Or anything in the changelog?

[anurag-git]

@lastmjs , I have seen many places code like make_shared<IntegerType>(160, IntegerType::Modifier::Address).

Please check with @chriseth , if we would like to change such occurrences.

[lastmjs]

I've got a PR up, need more direction @chriseth @vs77bb

[gitcoinbot]

@lastmjs Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[lastmjs]

I'm working on this and have a pr up but haven't heard back from anyone

[lastmjs]

@vs77bb @chriseth I'm nearly finished, but I need direction on what you'd like me to do to finish up.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__Work for 225.0 DAI (225.0 USD @ $1.0/DAI) has been submitted by__:

1. @lastmjs

@vs77bb please take a look at the submitted work:

• PR by @lastmjs

---

• Learn more on the Gitcoin Issue Details page
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $21,815.76 more funded OSS Work available on the Gitcoin Issue Explorer

[gitcoinbot]

@lastmjs Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[lastmjs]

I'm still working on this with the updated information I needed

[gitcoinbot]

@lastmjs Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[lastmjs]

I'll be getting to these updates soon

On Mon, Sep 10, 2018, 10:16 AM Gitcoin.co Bot [e-mail]
wrote:

@lastmjs https://github.com/lastmjs Hello from Gitcoin Core - are you
still working on this issue? Please submit a WIP PR or comment back within
the next 3 days or you will be removed from this ticket and it will be
returned to an ‘Open’ status. Please let us know if you have questions!

• warning (3 days)
• escalation to mods (6 days)

Funders only: Snooze warnings for 1 day
https://gitcoin.co/issue/ethereum/solidity/4502/777?snooze=1 | 3 days
https://gitcoin.co/issue/ethereum/solidity/4502/777?snooze=3 | 5 days
https://gitcoin.co/issue/ethereum/solidity/4502/777?snooze=5 | 10 days
https://gitcoin.co/issue/ethereum/solidity/4502/777?snooze=10 | 100 days
https://gitcoin.co/issue/ethereum/solidity/4502/777?snooze=100

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/ethereum/solidity/issues/4502#issuecomment-419971127,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGrSjwFfd7zlNs8LVEMj9DG2x179mwH-ks5uZpBEgaJpZM4VNewo
.

[vs77bb]

Thanks for the update @lastmjs

[lastmjs]

PR updated as asked on the PR thread.

[lastmjs]

The pull request has been merged

[lastmjs]

@vs77bb I submitted this work on Gitcoin a while ago, do I need to submit again now that it's complete?

[gitcoinbot]

Issue Status: 1. Open 2. Cancelled

---

__The funding of 225.0 DAI (225.0 USD @ $1.0/DAI) attached to this issue has been cancelled by the bounty submitter__

• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $26,316.05 more funded OSS Work available on the Gitcoin Issue Explorer

[gitcoinbot]

⚡️ A tip worth 225.00000 DAI (225.0 USD @ $1.0/DAI) has been granted to @lastmjs for this issue from @. ⚡️

Nice work @lastmjs! To redeem your tip, login to Gitcoin at https://gitcoin.co/explorer and select 'Claim Tip' from dropdown menu in the top right, or check your email for a link to the tip redemption page.

• $26541.05 in Funded OSS Work Available at: https://gitcoin.co/explorer
• Incentivize contributions to your repo: Send a Tip or Fund a PR
• No Email? Get help on the Gitcoin Slack

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__The funding of 225.0 DAI (225.0 USD @ $1.0/DAI) attached to this issue has been approved & issued.__

• Learn more on the Gitcoin Issue Details page
  
  
  
  • Questions? Checkout Gitcoin Help or the Gitcoin Slack
  
  
  • $26,316.05 more funded OSS Work available on the Gitcoin Issue Explorer
  
  

[lastmjs]

I'm trying to redeem the tip, but it doesn't work and when I look at the console I get this error: Uncaught Error: Invalid number of arguments to Solidity function

[lastmjs]

I wonder if it has something to do with the creator of the tip not putting their name in correctly or something

[vs77bb]

@lastmjs Sent this to our dev team; will message you on Gitcoin Slack when we figure it out 👍