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

fix: osnap improve how osnap form validates trarnsactions #123

Conversation

gsteenkamp89
Copy link

motivation

disables publish until ALL transactions in the builder are valid

Copy link

linear bot commented Feb 2, 2024

Copy link

vercel bot commented Feb 2, 2024

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 7, 2024 3:45pm
snapshot-goerli ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 7, 2024 3:45pm

} catch (error) {
return undefined;
return parseUnits('0', props.decimals).toString();
Copy link

Choose a reason for hiding this comment

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

do you actually need to call parse units on 0? cant you just put '0'?

Copy link

Choose a reason for hiding this comment

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

also im not sure if we should really be resetting input box to 0 if theres an error

emit('updateTransaction', transaction);
return;
if (!isTransactionValid) {
console.warn('invalid transaction');
}
} catch (error) {
Copy link

Choose a reason for hiding this comment

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

what happens here? if transaction errors the parent never sees this is invalid

Copy link
Author

Choose a reason for hiding this comment

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

can remove conditional entirely

@@ -24,7 +24,7 @@ const emit = defineEmits<{

const to = ref(props.transaction.to ?? '');
const isToValid = computed(() => {
return to.value === '' || isAddress(to.value);
return to.value !== '' && isAddress(to.value);
Copy link
Author

Choose a reason for hiding this comment

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

oh i need to revert this change, since value should be allowed to be 0

Comment on lines +61 to +63
emit('updateTransaction', transaction);
} catch {
props.setTransactionAsInvalid();
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 essentially what I'm doing in each transaction type.
If validation fails, we throw, then in the catch block we keep the data the same upstream, but we add an invalid flag.
the form doesn't lose it's state. so as soon as the user inputs data that doesn't cause the create<TransactionType> to throw, we can update state upstream with the formatted Tx and set it to be valid

groninge01 and others added 2 commits February 7, 2024 10:53
* lowercase useraddress from url

* lowercase id here
@gsteenkamp89 gsteenkamp89 changed the base branch from gerhard/uma-2153-ui-render-the-tenderly-simulation-button-in-the-tx-builder to master February 7, 2024 15:47
@gsteenkamp89 gsteenkamp89 changed the base branch from master to gerhard/uma-2153-ui-render-the-tenderly-simulation-button-in-the-tx-builder February 7, 2024 15:48
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.

4 participants