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

4 changes: 2 additions & 2 deletions src/components/Ui/UiInput.vue
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const props = defineProps<{

const emit = defineEmits(['update:modelValue', 'blur']);

function handleInput(e) {
const input = e.target.value;
function handleInput(e: Event) {
const input = (e.target as HTMLInputElement).value;
if (props.number) {
return emit('update:modelValue', !input ? undefined : parseFloat(input));
}
Expand Down
2 changes: 2 additions & 0 deletions src/composables/useFormSpaceProposal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useStorage } from '@vueuse/core';
import { clone } from '@snapshot-labs/snapshot.js/src/utils';
import schemas from '@snapshot-labs/snapshot.js/src/schemas';
import { validateForm } from '@/helpers/validation';
import { OsnapPluginData } from '@/plugins/oSnap/types';

interface ProposalForm {
name: string;
Expand All @@ -15,6 +16,7 @@ interface ProposalForm {
metadata: {
plugins: {
safeSnap?: { valid: boolean };
oSnap?: OsnapPluginData;
};
};
}
Expand Down
15 changes: 11 additions & 4 deletions src/plugins/oSnap/Create.vue
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ import {
Transaction
} from './types';
import {
allTransactionsValid,
getGnosisSafeBalances,
getGnosisSafeCollectibles,
getIsOsnapEnabled,
getModuleAddressForTreasury
getModuleAddressForTreasury,
validateOsnapTransaction
} from './utils';
import OsnapMarketingWidget from './components/OsnapMarketingWidget.vue';

Expand Down Expand Up @@ -46,19 +48,24 @@ const collectables = ref<NFT[]>([]);
function addTransaction(transaction: Transaction) {
if (newPluginData.value.safe === null) return;
newPluginData.value.safe.transactions.push(transaction);
newPluginData.value.safe.isValid = allTransactionsValid(
newPluginData.value.safe.transactions
);
update(newPluginData.value);
}

function removeTransaction(transactionIndex: number) {
if (!newPluginData.value.safe) return;

newPluginData.value.safe.transactions.splice(transactionIndex, 1);
update(newPluginData.value);
}

function updateTransaction(transaction: Transaction, transactionIndex: number) {
if (!newPluginData.value.safe) return;
newPluginData.value.safe.transactions[transactionIndex] = transaction;
newPluginData.value.safe.isValid = allTransactionsValid(
newPluginData.value.safe.transactions
);
update(newPluginData.value);
}

Expand All @@ -76,7 +83,6 @@ async function fetchBalances(network: Network, safeAddress: string) {
if (!safeAddress) {
return [];
}

try {
const balances = await getGnosisSafeBalances(network, safeAddress);

Expand Down Expand Up @@ -233,7 +239,8 @@ onMounted(async () => {
<div class="rounded-2xl border p-4 text-md">
<h2 class="mb-2">Warning: Multiple oSnap enabled plugins detected</h2>
<p class="mb-2">
For best experience using oSnap, please remove the SafeSnap plugin from your space.
For best experience using oSnap, please remove the SafeSnap plugin from
your space.
</p>
</div>
</template>
Expand Down
23 changes: 18 additions & 5 deletions src/plugins/oSnap/components/Input/Amount.vue
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
<script setup lang="ts">
import { formatUnits, parseUnits } from '@ethersproject/units';
import { amountPositive } from '../../utils';

const props = defineProps<{
modelValue: string;
label: string;
decimals: number | undefined;
enforcePositiveValue?: boolean;
}>();

const emit = defineEmits<{
'update:modelValue': [value: string];
}>();
Expand All @@ -16,24 +19,34 @@ const dirty = ref(false);

const format = (amount: string) => {
try {
return parseUnits(amount, props.decimals).toString();
} catch (error) {
return undefined;
// empty string throws
const parsed = parseUnits(amount, props.decimals).toString();
return parsed;
} catch {
return '0';
}
};

const handleInput = () => {
dirty.value = true;
const value = format(input.value);
isValid.value = !!value;
emit('update:modelValue', value ?? '');
// empty string value will throw error being converted to BigNumber
emit('update:modelValue', value ?? '0');
};

onMounted(() => {
if (props.modelValue) {
input.value = formatUnits(props.modelValue, props.decimals);
}
});

watch(input, () => {
const value = format(input.value);
isValid.value = !!value;
if (props.enforcePositiveValue) {
isValid.value = amountPositive(input.value);
}
});
</script>

<template>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ import { ContractInteractionTransaction, Network } from '../../types';
import {
createContractInteractionTransaction,
getABIWriteFunctions,
getContractABI,
validateTransaction
getContractABI
} from '../../utils';
import AddressInput from '../Input/Address.vue';
import MethodParameterInput from '../Input/MethodParameter.vue';

const props = defineProps<{
network: Network;
transaction: ContractInteractionTransaction;
setTransactionAsInvalid: () => void;
}>();

const emit = defineEmits<{
Expand All @@ -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

});
const abi = ref(props.transaction.abi ?? '');
const isAbiValid = ref(true);
Expand All @@ -40,22 +40,27 @@ const selectedMethod = computed(
const parameters = ref<string[]>([]);

function updateTransaction() {
if (!isValueValid || !isToValid || !isAbiValid) return;
try {
if (!isToValid) {
throw new Error('"TO" address invalid');
}
if (!isAbiValid) {
throw new Error('ABI invalid');
}
if (!isValueValid) {
throw new Error('Value invalid');
}
// throws is method params are invalid
const transaction = createContractInteractionTransaction({
to: to.value,
value: value.value,
abi: abi.value,
method: selectedMethod.value,
parameters: parameters.value
});

if (validateTransaction(transaction)) {
emit('updateTransaction', transaction);
return;
}
} catch (error) {
console.warn('ContractInteraction - Invalid Transaction:',error);
emit('updateTransaction', transaction);
} catch {
props.setTransactionAsInvalid();
Comment on lines +61 to +63
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

}
}

Expand All @@ -73,7 +78,7 @@ function updateMethod(methodName: string) {
function updateAbi(newAbi: string) {
abi.value = newAbi;
methods.value = [];

updateTransaction();
try {
methods.value = getABIWriteFunctions(abi.value);
isAbiValid.value = true;
Expand All @@ -82,26 +87,25 @@ function updateAbi(newAbi: string) {
isAbiValid.value = false;
console.warn('error extracting useful methods', error);
}
updateTransaction();
}

async function updateAddress() {
updateTransaction();
const result = await getContractABI(props.network, to.value);
if (result && result !== abi.value) {
updateAbi(result);
}
updateTransaction();
}

function updateValue(newValue: string) {
value.value = newValue;
updateTransaction();
try {
parseAmount(newValue);
isValueValid.value = true;
} catch (error) {
isValueValid.value = false;
}
updateTransaction();
}
</script>

Expand Down Expand Up @@ -137,8 +141,11 @@ function updateValue(newValue: string) {
</option>
</UiSelect>

<div v-if="selectedMethod && selectedMethod.inputs.length">
<div class="divider"></div>
<div
v-if="selectedMethod && selectedMethod.inputs.length"
class="flex flex-col gap-2"
>
<div class="divider h-[1px] bg-skin-border my-3" />

<MethodParameterInput
v-for="(input, index) in selectedMethod.inputs"
Expand Down
12 changes: 12 additions & 0 deletions src/plugins/oSnap/components/TransactionBuilder/Transaction.vue
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ function updateTransaction(transaction: TTransaction) {
newTransaction.value = transaction;
emit('updateTransaction', newTransaction.value, props.transactionIndex);
}

function setTransactionAsInvalid() {
const tx: TTransaction = {
...newTransaction.value,
isValid: false
};
emit('updateTransaction', tx, props.transactionIndex);
}
</script>

<template>
Expand All @@ -66,6 +74,7 @@ function updateTransaction(transaction: TTransaction) {
v-if="transaction.type === 'contractInteraction'"
:transaction="newTransaction as ContractInteractionTransaction"
:network="network"
:setTransactionAsInvalid="setTransactionAsInvalid"
@update-transaction="updateTransaction"
/>

Expand All @@ -74,6 +83,7 @@ function updateTransaction(transaction: TTransaction) {
:network="network"
:tokens="tokens"
:transaction="newTransaction as TransferFundsTransaction"
:setTransactionAsInvalid="setTransactionAsInvalid"
@update-transaction="updateTransaction"
/>

Expand All @@ -83,12 +93,14 @@ function updateTransaction(transaction: TTransaction) {
:safe-address="safeAddress"
:collectables="collectables"
:transaction="newTransaction as TransferNftTransaction"
:setTransactionAsInvalid="setTransactionAsInvalid"
@update-transaction="updateTransaction"
/>

<RawTransaction
v-if="transaction.type === 'raw'"
:transaction="newTransaction as TRawTransaction"
:setTransactionAsInvalid="setTransactionAsInvalid"
@update-transaction="updateTransaction"
/>
</div>
Expand Down
42 changes: 23 additions & 19 deletions src/plugins/oSnap/components/TransactionBuilder/TransferFunds.vue
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
<script setup lang="ts">
import { ETH_CONTRACT } from '@/helpers/constants';
import { shorten } from '@/helpers/utils';
import { isAddress } from '@ethersproject/address';
import { isBigNumberish } from '@ethersproject/bignumber/lib/bignumber';
import { Network, Token, TransferFundsTransaction } from '../../types';
import {
createTransferFundsTransaction,
getERC20TokenTransferTransactionData,
getNativeAsset,
validateTransaction
isTransferFundsValid,
amountPositive
} from '../../utils';
import AddressInput from '../Input/Address.vue';
import AmountInput from '../Input/Amount.vue';
Expand All @@ -18,6 +17,7 @@ const props = defineProps<{
network: Network;
tokens: Token[];
transaction: TransferFundsTransaction;
setTransactionAsInvalid: () => void;
}>();

const emit = defineEmits<{
Expand All @@ -28,40 +28,43 @@ const nativeAsset = getNativeAsset(props.network);
const amount = ref(props.transaction.amount ?? '');
const recipient = ref(props.transaction.recipient ?? '');
const tokens = ref<Token[]>([nativeAsset, ...props.tokens]);

const selectedTokenAddress = ref<Token['address']>(
props.transaction?.token?.address ?? 'main'
);

const selectedToken = computed(
() =>
tokens.value.find(token => token.address === selectedTokenAddress.value) ??
nativeAsset
);
const selectedTokenIsNative = computed(
() => selectedToken.value?.address === 'main'
);

const isTokenModalOpen = ref(false);

function updateTransaction() {
if (!isBigNumberish(amount.value) || !isAddress(recipient.value)) return;

try {
const data = selectedTokenIsNative.value
? '0x'
: getERC20TokenTransferTransactionData(recipient.value, amount.value);

if (
!isTransferFundsValid({
amount: amount.value,
recipient: recipient.value,
token: selectedToken.value
})
) {
throw new Error('Validation error');
}
const data =
selectedToken.value.address === 'main'
? '0x'
: getERC20TokenTransferTransactionData(recipient.value, amount.value);
const transaction = createTransferFundsTransaction({
data,
amount: amount.value,
recipient: recipient.value,
token: selectedToken.value
});
const isTransactionValid = validateTransaction(transaction);
if (isTransactionValid) {
emit('updateTransaction', transaction);
return;
}
} catch (error) {
console.warn('invalid transaction', error);
emit('updateTransaction', transaction);
} catch {
props.setTransactionAsInvalid();
}
}

Expand Down Expand Up @@ -104,6 +107,7 @@ watch(selectedTokenAddress, updateTransaction);
<div class="space-y-2">
<AddressInput v-model="recipient" :label="$t('safeSnap.to')" />
<AmountInput
:enforcePositiveValue="true"
:key="selectedToken?.decimals"
v-model="amount"
:label="$t('safeSnap.amount')"
Expand Down
Loading
Loading