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

[Web Stacks Wallet] Contract post conditions replaced by standard conditions #1013

Closed
friedger opened this issue Feb 22, 2021 · 9 comments
Closed

Comments

@friedger
Copy link
Contributor

@friedger friedger commented Feb 22, 2021

A tx with contract post condition and standard post conditions are shown in the same way. See screenshot:
Screenshot from 2021-02-22 09-08-38

The contract post condition should mention the contract

@friedger friedger changed the title [Web Stacks Wallet] Contract post conditions shown as standard conditions [Web Stacks Wallet] Contract post conditions replaced by standard conditions Feb 22, 2021
@friedger
Copy link
Contributor Author

@friedger friedger commented Feb 22, 2021

It looks like the contract post condition is replaced by a standard post condition, making it impossible to transfer stx from a contract.
The contract call code below results in two standard post conditions:

Code:
Screenshot from 2021-02-22 09-21-54

Post conditions in explorer:
Screenshot from 2021-02-22 09-19-55

@hstove
Copy link
Collaborator

@hstove hstove commented Mar 11, 2021

It's actually worse than this - the current post conditions logic assumed that post conditions could only be applied to the tx-sender. Jude recently updated some wording in this SIP that clears this up. So, the Stacks Wallet actually overwrites every post condition with the current account's STX address.

Related: #969

@314159265359879
Copy link

@314159265359879 314159265359879 commented Mar 16, 2021

Can fixing this get a higher priority. This is making it impossible to test ft-transfers using the explorer sandbox or any other app.

@hstove
Copy link
Collaborator

@hstove hstove commented Mar 16, 2021

I agree that it's important, and we have this bucketed in our highest-priority milestone. But, for testing purposes, you don't have to add post conditions for addresses other than the sender. You should in production, but to get the tx to go through, you only need to add them for the sender.

@friedger
Copy link
Contributor Author

@friedger friedger commented Mar 17, 2021

Removing the contract condition does not help: https://explorer.stacks.co/txid/0xa14334f26ff547516135b5510809dee33db5797c836f8954c0ed45c2fb51f87b?chain=testnet

On https://speed-spend.org/poxlite it is not possible to redeem stingers because the post condition for the sender of STX, i.e. the smart contract can't be specified in the stacks web wallet.

friedger/speed-spend@c3b30b6

@hstove
Copy link
Collaborator

@hstove hstove commented Mar 17, 2021

What I'm suggesting is that it would be better for that tx to just have no post conditions at all. Because the tx-sender is not actually transferring any assets, the tx will be successful, even in Deny mode.

@unclemantis
Copy link

@unclemantis unclemantis commented Mar 17, 2021

What I'm suggesting is that it would be better for that tx to just have no post conditions at all. Because the tx-sender is not actually transferring any assets, the tx will be successful, even in Deny mode.

Can you be more clear?

@unclemantis
Copy link

@unclemantis unclemantis commented Mar 17, 2021

What I'm suggesting is that it would be better for that tx to just have no post conditions at all. Because the tx-sender is not actually transferring any assets, the tx will be successful, even in Deny mode.

Wait. I will try that. Thanks.

@hstove
Copy link
Collaborator

@hstove hstove commented Mar 30, 2021

Fixed and released in version 2.3.0 👍

@hstove hstove closed this Mar 30, 2021
👟 UserX Kanban automation moved this from 📥 Backlog to 🚀 Done Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
👟 UserX Kanban
🚀 Done Done
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants