8000 total supply is not changing on deposit/withdrawal · Issue #6 · dapphub/ds-eth-token · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

total supply is not changing on deposit/withdrawal #6

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.

8000
Sign up for GitHub

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

Jump to bottom
Open
Georgi87 opened this issue May 10, 2017 · 10 comments
Open

total supply is not changing on deposit/withdrawal #6

Georgi87 opened this issue May 10, 2017 · 10 comments

Comments

@Georgi87
Copy link

I would have expected totalSupply to keep track of the total ETH wrapper tokens. https://github.com/dapphub/ds-eth-token/blob/master/src/eth_wrapper.sol#L40 is increasing a user's balance without updating the totalSupply.
@gbalabasquer
Copy link
Contributor

Hi @Georgi87 look at the implementation of totalSupply

function totalSupply() constant returns (uint supply) {
        return this.balance;
    }

It is returning this.balance which is the real ETH amount deposited in the contract. In this case is the same than W-ETH quantity.

@Georgi87
Copy link
Author

Ok, that makes sense. Thanks for pointing it out @gbalabasquer

@gbalabasquer
Copy link
Contributor

No problem!

@Georgi87
Copy link
Author

It is maybe still no ideal as there are ways to change the contract balance without triggering any contract function using selfdestruct(contract_address). So the totalSupply could be higher than the sum of all balances.

@gbalabasquer
Copy link
Contributor

I'm not getting you. Can you give me an exact example of it?
Because if you destroy a contract that owns certain quantity of W-ETH, the quantity of W-ETH continues assigned to that address regardless the contract was destroyed or not.

@Georgi87
Copy link
Author
Georgi87 commented May 10, 2017 •
edited
Loading

Assuming I have a contract with an Ether balance > 0 and this contract has a selfdescruct function. I can call selfdestruct(eth_wrapper_token_address) sending all ETH to the eth wrapper contract without calling any function in the wrapper contract. Also the fallback function is not triggered. totalSupply() will return an increased balance but the ETH doesn't belong to anytone in the balance mapping. It is a minor thing.

@gbalabasquer
Copy link
Contributor

Oh yes I get what you mean now. It is a valid point, we will check in the next review.
Thanks for pointing this out.

@nmushegian
Copy link
Member

@Georgi87 Thanks for this observation, this is actually a "major" issue given how important this building block is. We do have 1 redeploy planned before we commit fully to an eth wrapper contract, so this is a really fortunate find.

@gbalabasquer
Copy link
Contributor

@nmushegian I'm reopening this issue and will make the change tomorrow.

@gbalabasquer gbalabasquer reopened this May 10, 2017
@gbalabasquer
Copy link
Contributor

Fixed in #8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Georgi87 @nmushegian @gbalabasquer
0