When did the Grin++ wallet code last get audited?

Seems like there is a serious problem with the wallet. “This sounds like the same issue I had and made a rant/flame about. Eventually @davidtavarez worked with me for about a month on the issue before just giving me grin out of his pocket. He did not fix the issue. Sounds like this bug will remain and Grin++ is unstable / unsafe to use, correct me if I’m wrong, thanks.”. Lost Grin coins while sending to TradeOgre - #2 by vegycslol

2 Likes

David replied in the thread you linked to that he is working on a fix. That was 3 days ago.

The Grin++ code was audited 3 days ago?

No, but this problem existed for a long time, I think also after the last audit. It was only discovered later though, otherwise it would have been included and fixed in some of the previous funding requests.
The fact that it only got discovered recently means it is an edge case, not a problem that most users will run into. Still, any case of funds getting into limbo is bad and should fixed high priority even if they are rarely happening because they can be easily misused to create FUD.

FUD implies it’s a user-perception problem. It’s not.

1 Like

Not exactly. FUD means that something can be used to create Fear Uncertainty and Doub by for example exagerating a problems seriousness or frequency or by taking it out of context. Not saying the problem is not real, but neither should it be made larger than it is (e.g. a bug reported by two users does not warrant a code audit, but perhaps if the problem is complex @david can be asked for a review of the bug fix).

Are you trying to throw a straw man here? I asked “When did the Grin++ wallet code last get audited?”

You mention 2 people have reported the issue. Out of how many users? A few 100, a few 1000? Out of those users, how many actually send to exchanges? I know people that withdraw to their wallets only.

The fact is code should be audited on a regular basis, especially when dealing with money.

From Trust Wallet:

At Trust Wallet, we put a strong emphasis on security and keeping our community informed. We collaborate with top security specialists, including our internal security teams as well as reputable third-party auditing firms, to perform frequent audits on our application and infrastructure. This ensures that everyone who uses Trust Wallet can have confidence in the safety and dependability of our products.

We make significant investments in our protocols and our team of security experts who carry out over 30 audits annually, encompassing both the application and infrastructure aspects. Through these regular security audits, we’re able to quickly identify and remediate any potential vulnerabilities before they can be exploited.

Be sure to check back regularly as we share periodic security audits:

I have recommended the wallet to a number of people, and it will likely be the main wallet used by many other people in the future. If there was more than one audit, would that hurt?

I see 0 reason why someone merely asking when an audit was carried out and pointing out a clearly serious issue of someone losing their funds should be considered as fud. Let’s not forget that David also refunded them out of his own pocket. Is this the norm now?

There are far more experienced developers on here than I am, so maybe they can chime in as to when a code should be audited and how often, and if the issues above would warrant one.

1 Like

I have no idea why he is throwing around accusations!

The bug has existed since May (at least) and involves GRIN being in a locked state. It’s been reported at least 3 times in the forum. Not everyone reports bugs like this either. You’re downplaying the issue.

user-funds get lost due to bugs is a huge issue. The amount lost doesnt matter. In one case it could be 1 Grin, in another case it could affect a billion Grin.

If an unknown corner-case can make your grin dissapear from your wallet indefinetly this means the the wallet is not save. You just cannot use an unsave wallet. So even the mere existence of a corner case that can potentiially make grin dissapear renders the Grin++ useless for most users, at least the ones that would mind their money simply vaporizing for no apparent reason.

If there is an unresolved bug leading to missing funds, the download-page of Grin++ should at least warn potential Users about the known issue, until the bug is fixed.

1 Like

Can you (or someone else) share how they know the coins are lost indefinitely? Because i fail to see how this could be the case. In one example a user sent coins to tradeogre who never got them (for whatever reason). The user had a corrupted state in his grin++ wallet. Now there are 2 options which imo should work:

  1. clean grin++ data, restore wallet and sync from beginning (if cleaning works in grin++)
  2. sync rust grin and restore the wallet

If those 2 scenarios work (which i can’t see why they wouldn’t if tradeogre actually never received the coins - they have no reason to lie) then why is everyone making such a big deal out of it? Sure it’s a bit unpleasant for the user, but he doesn’t lose his coins. It seems to me like many people here underestimate the difficulty of writing bug free code. It’s also hard to sometimes figure out why certain cases happen.

If you can’t explain why they’re actually lost then please stop saying that they’re. If you can explain then I’m eagerly awaiting this explanation.

In case the coins are locked, it either means the broadcast failed, or subsequently Dandelion got disrupted, meaning the transaction needs to be re-broadcasted.
In any case, it does not mean funds get lost, just that the wallet state did not get properly updated. A user unfriendly bug since it would require current users to wait for a retry/resend/re-broadcast button to be implemented in Grin++ or them removing the Grin++ temp folder which hold the wallet state, losing their transaction history but unlocking the coins.
In such a case, loading the wallet in another wallet would also unlock the funds.

What confused me is that it was reported that loading in grin-wallet did not work. In such a case the issue might be different. E.g. user confusion when restoring a wallet because they do not see the history of outputs they already spend.

The only case I can think of that funds actually got sort of lost is if a self spend is wrongly performed, e.g. with a wrongly formatted key. But even then, it does not mean funds got lost for real, just that a bug locks them with a wrong key. In any case, let’s just wait for @davidtavarez to do his magic and solve it,:magic_wand:.

2 Likes

Becuz your coins / money stays in limbo, you need to struggle with getting your coins back or convince that exchange has your coins many days.
This is not the case only for tradeogre, gate.io same. And ironbelly that i used, i had similar experience with gate.io. You need to rebroadcast the tx.

:point_up_2:

This is extremely anti user, ironbelly doesnt have this button also afaik. It is problem not only for sending but receiving from exchanges. I waited 13 days to receive my coins from gate.io.
This is the ultimate FUD for a new average user.

I agree that in grin you need to have a rebroadcast button because the transaction is only sent to one person which can drop it (iirc grin-wallet cli has a “repost” command). But that’s not a “coins lost” problem, that would be a way bigger issue.

1 Like

Button is not solving problem, especially on exchanges side, we can say its also anti-exchange (now we can realise why we lost so much exchanges and not adding any new). Its problem on protocol level, I guess it can be useful to disable/make optional usage of Dandelion since its not stable before problem will be solved on protocol level.

2 Likes

Not a protocol problem. Repost button should do the trick. Also, wallet can check if a broadcasted transaction did not complete after broadcasting in a certain time and either prompt the user or auto broadcast again. That solves the problem which is still an edge case. Disabling Dandelion would needlessly reduce default privacy for all.

1 Like

Dandelion is part of protocol here, we can use Tor to hide IP I guess. Wallet can check, but you need to keep it open for some time to check status, or check on reopen, what is also not an option for cli/exchange I guess, I mean it should be possible to fix it on node/protocol level to rebroadcast tx when some node on Dandelion phase got failed, from what I saw some Grin++ nodes constantly connected/disconnected to my node as peers in a loop, I guess it could be a part of problem.

If an exchange would be the initiator of the transaction (RSR) and as such the one broadcasting, there would be no problem. If I am not mistaken grin-wallet can bypass dandelion.
So in that case an exchange would be 100% in control and have complete certainty whether a transaction got properly broadcasted or not. Optionally the exchange could use Dandelion for privacy and perform a repost in the broadcasted transaction did not appear on chain after a few hours.

1 Like

In current implementation problem for exchange now is to check if transaction is not getting any confirmations for long period, so they need to cancel/rebroadcast it manually.

grin-wallet can not do anything with Dandelion, its option is from the node config:

This is why I suggested to check status of such txs at node level to rebroadcast it from there if some problem happened at any phase, it can be ability to setup timeout.

Current task/problem for me is to setup test environment with reduced amount of nodes to reproduce this issue at regular basis to check how it can work…

1 Like

Frankly i didnt understand if that unconfirmed tx is a protocol level or grin++ problem.
Core devs know better @Yeastplume , @oryhp , if he is around @david.