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

Gerhard/uma 2479 ensure osnap form input types working correctly #158

Conversation

gsteenkamp89
Copy link

@gsteenkamp89 gsteenkamp89 commented Apr 10, 2024

motivation

Currently our transaction builder is not parsing or validating array or tuple param types AT ALL!
This PR seeks to properly parse array inputs and validate members of those arrays.
both array and tuple inputs require an array (square brackets) in the input field.
but array param members are always the same type so if the type is uint256[] then we validate each member of teh array as a uint156
but tuples are more complex since they generally represent struct values, so a tuple's type can be [uint256,address,bool]

To test

  • New proposal => use osnap => new tx => contract Interaction
  • import this abi
  • select function requestPrice
  • test input field for argument requestSettings, the tuple type is [bool,bool,bool,Int,Int]

Or find any ABI with function parameters of type Array or Tuple and test the input.

Copy link

linear bot commented Apr 10, 2024

Copy link

vercel bot commented Apr 10, 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 Apr 12, 2024 1:44pm
snapshot-goerli ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 12, 2024 1:44pm

@gsteenkamp89 gsteenkamp89 marked this pull request as draft April 10, 2024 17:51
@gsteenkamp89 gsteenkamp89 changed the base branch from master to gerhard/uma-2431-figure-out-how-to-get-safe-transactions-into-snapshot-osnap April 10, 2024 18:51
@gsteenkamp89 gsteenkamp89 changed the base branch from gerhard/uma-2431-figure-out-how-to-get-safe-transactions-into-snapshot-osnap to master April 10, 2024 18:51
@gsteenkamp89 gsteenkamp89 marked this pull request as ready for review April 11, 2024 16:02
@gsteenkamp89 gsteenkamp89 changed the base branch from master to gerhard/uma-2431-figure-out-how-to-get-safe-transactions-into-snapshot-osnap April 11, 2024 16:11
@gsteenkamp89 gsteenkamp89 changed the base branch from gerhard/uma-2431-figure-out-how-to-get-safe-transactions-into-snapshot-osnap to uma-add-safe-transaction-file-import-to-osnap-plugin April 11, 2024 16:17
…nto gerhard/uma-2479-ensure-osnap-form-input-types-working-correctly
…e builder (#4649)

* feat: allow safe json upload into osnap tx builder

Signed-off-by: david <[email protected]>

* parse batchfile & populate input fields with values

* show value & to fields

* improve input

* improve file input error handling and styling

* refactor

* refactor

* minor bug fix

* allow quick fix for short bytes32

* align icon better

* make file input own component

* import file once and initialize multiple safeImport transactions

* check if safe address & chainId match

* small fixes

* clean up comments

* better border sie for file input

* clear input element value to allow recurring upload

* fix quick fix

* handle multiple files

* better error message

* emit from watcher

* fix: use upstream state only

* clean up

* do byteslike check last

* fixes

* add default label

* fix: remove component level tx state

* Merge branch 'master' into uma-add-safe-transaction-file-import-to-osnap-plugin

* validate address input on blur

* clean up input css

* improve bytes32 error message

* replace readOnly with hidden, casing change

---------

Signed-off-by: david <[email protected]>
Co-authored-by: david <[email protected]>
Co-authored-by: Chaitanya <[email protected]>
@gsteenkamp89
Copy link
Author

gsteenkamp89 commented Apr 12, 2024

@daywiss I've added better checks for Integers as well. We now do range checks for all integer types. I noticed I was able to insert values for uint32 that were out of bounds and the app said it was valid. so this i now fixed.

max value for uint32
Screenshot 2024-04-12 at 13 41 06

max + 1
Screenshot 2024-04-12 at 13 41 14

@gsteenkamp89 gsteenkamp89 changed the base branch from uma-add-safe-transaction-file-import-to-osnap-plugin to master April 12, 2024 13:25
@gsteenkamp89 gsteenkamp89 changed the base branch from master to uma-add-safe-transaction-file-import-to-osnap-plugin April 12, 2024 13:27
@gsteenkamp89 gsteenkamp89 force-pushed the gerhard/uma-2479-ensure-osnap-form-input-types-working-correctly branch from 2dbae98 to 53f5681 Compare April 12, 2024 13:33
@gsteenkamp89 gsteenkamp89 changed the base branch from uma-add-safe-transaction-file-import-to-osnap-plugin to master April 12, 2024 13:33
@gsteenkamp89
Copy link
Author

closing in favor of #161

@gsteenkamp89 gsteenkamp89 removed the request for review from daywiss April 12, 2024 14:18
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.

2 participants