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

feat: tenderly simulation button in transaction builder #120

Conversation

gsteenkamp89
Copy link

@gsteenkamp89 gsteenkamp89 commented Dec 22, 2023

Motivation

We want to simulate transaction before the proposal gets posted on chain.

screenshots

Screenshot 2024-01-30 at 15 49 13 Screenshot 2024-01-30 at 15 49 17 Screenshot 2024-01-30 at 15 49 20

Copy link

linear bot commented Dec 22, 2023

Copy link

vercel bot commented Dec 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
snapshot ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 21, 2024 2:17pm
snapshot-goerli ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 21, 2024 2:17pm

Comment on lines +17 to +22
if (!safe) {
throw new Error('No safe data');
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't be null at this point in the flow but we need to make ts happy

Comment on lines 17 to 18
green: '#21b66f',
red: '#ff3856'
green: 'hsl(var(--green) / <alpha-value>)',
red: 'hsl(var(--red) / <alpha-value>)'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is backward compatible. any existing references to green or red will just default to an alpha value of 1


interface ResultUrl {
url: string; // This is the URL to the simulation result page (public or private).
public: boolean; // This is false if the project is not publicly accessible.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for frontend, we might also validate that this is true, otherwise the returned url will not be accessible. Normally API is expected to run with public project, but its better to check it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah great thanks Reinis! I think, correct me if I'm wring, but we've configured the API to always be using UMA's project since we're always interacting with the OG. Will this ever change? I suppose it might in which case we'd probably have to show some sort of warning that the link might not be accessible to the user.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, API uses public Tenderly project credentials, but still worth checking as this can be changed in project settings

@daywiss
Copy link

daywiss commented Jan 31, 2024

im not sure how to do this, but we may need to pr this to the snapshot repo rather than uma/master

Copy link

@daywiss daywiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only suggestion right now is to change reset to "reset simulation" which i think you handle in the gas pr

@daywiss
Copy link

daywiss commented Feb 14, 2024

getting some strange behavior with the proxy vs implementation logic.

  1. create new proposal, https://snapshot-goerli-ijoq16vqw-uma.vercel.app/#/osnap-v2.eth/create
  2. select contract interaction: 0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359
  3. select implementation
  4. put in spender address local wallet
  5. set value to 0, see that simluation succeeds:
    image
  6. cut "To" and paste it back to retrigger proxy/implementation modal
  7. select implementation ( again )
  8. see that you cannot publish or simulate with a valid form:
    image

@daywiss
Copy link

daywiss commented Feb 22, 2024

accepted upstream

@daywiss daywiss closed this Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants