SNIP-25 allowance queries allow anyone to query by signing a permit. about snip20-reference-impl HOT 8 CLOSED

Thanks for reporting this! You're 100% right, there's a missing check there for spender/owner. I'll also advise any devs who might have deployed contracts from this version to look out of the issue.

I added this as a fix for the issue #84 - I wanted to avoid changing interfaces, even though the owner/spender params could be omitted, breaking interfaces which apps may rely on is never fun.

from snip20-reference-impl.

Great! The fix looks good to me. I checked all the contracts uploaded to mainnet and there are currently 0 instances uploaded that use the code at that commit (which is still the latest on master). Of course, that doesn't disqualify the possibility that someone could've forked and changed the code which would result in a different code hash. But it seems to me that now is the time to change the interface, as otherwise, it will remain misleading. If I not mistaken the SNIP-25 standard isn't even official yet?

from snip20-reference-impl.

Great! The fix looks good to me. I checked all the contracts uploaded to mainnet and there are currently 0 instances uploaded that use the code at that commit (which is still the latest on master). Of course, that doesn't disqualify the possibility that someone could've forked and changed the code which would result in a different code hash. But it seems to me that now is the time to change the interface, as otherwise, it will remain misleading. If I not mistaken the SNIP-25 standard isn't even official yet?

I think there should be some tokens which use SNIP-25 already as Shade has launched their migration tool weeks ago. (https://blog.shadeprotocol.io/snip-token-migration-launching/)

SHD
IST
USK
KUJI
ATOM
JUNO
OSMO
stJUNO
stATOM
stOSMO
STRIDE
BLD
LUNA

I don't think this was ever planned to be part of SNIP-25. It will probably go in as SNIP-26.

from snip20-reference-impl.

thanks for catching this @aakamenov and fixing it @Cashmaney! sorry I didn't notice this issue earlier.

from snip20-reference-impl.

Yeah, everyone was rushing to make sure new tokens fix previous issues and have as much functionality as possible which always adds some risk. In the future we def need to make sure to keep new features standardized.

Also merged the changes from the PR. I'm going to keep the interfaces as-is for now - we can make the owner parameter optional later, but if we want to do that, we should make sure that the regular allowance queries reflect that and that the toolkit/secretjs etc all gets updated to reflect those changes.

I'm going to go ahead and close this issue for now. I opened an issue in the SNIPs repo to add a doc for SNIP-25, I'll try and populate that tomorrow with all the info, and if anyone wants to continue the discussion about the parameters we can do that over there.

Lastly, I'll ping Shade about the issue so they can be aware of it until contracts are upgradable. Thanks for the report @aakamenov

from snip20-reference-impl.

⌛

from snip20-reference-impl.

Great! The fix looks good to me. I checked all the contracts uploaded to mainnet and there are currently 0 instances uploaded that use the code at that commit (which is still the latest on master). Of course, that doesn't disqualify the possibility that someone could've forked and changed the code which would result in a different code hash. But it seems to me that now is the time to change the interface, as otherwise, it will remain misleading. If I not mistaken the SNIP-25 standard isn't even official yet?

I think there should be some tokens which use SNIP-25 already as Shade has launched their migration tool weeks ago. (https://blog.shadeprotocol.io/snip-token-migration-launching/)

SHD
IST
USK
KUJI
ATOM
JUNO
OSMO
stJUNO
stATOM
stOSMO
STRIDE
BLD
LUNA

from snip20-reference-impl.

hmmm, it does look the queries were implemented in the Shade SNIP-25s. You can tell by sending an incorrect message type and parsing the error message.

Not that it would necessarily have meant catching this bug, but I think it would be a good idea to keep updates like this in a separate branch from master until the SNIPs are officially written-up. (though I see now there's no official tag release, yet, for snip25 so I guess the latest version in master should be considered beta).

edit-- made a small PR to add a note about the releases on README: #85

from snip20-reference-impl.