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

Validation fixes #161

Open
wants to merge 41 commits into
base: gerhard/uma-2547-adding-unsupported-treasury-crashes-proposal-page
Choose a base branch
from

Conversation

gsteenkamp89
Copy link

@gsteenkamp89 gsteenkamp89 commented Apr 12, 2024

rebase too time consuming, commits cherry-picked from original PR

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.

@daywiss I've added better checks for Integers as well. We now do range checks for all integer types. I noticed this bug because I was able to insert values for uint32 that were out of bounds and the app didn't catch it.

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

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

Copy link

vercel bot commented Apr 12, 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 May 28, 2024 11:09am
snapshot-goerli ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 28, 2024 11:09am
snapshot-sepolia ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 28, 2024 11:09am

Comment on lines +187 to +220
function isValidInt(value: string, type: Integer) {
// check if is number like
if (!isBigNumberish(value)) {
return false;
}

const unsigned = type.startsWith('uint');
const signed = type.startsWith('int');
if (!unsigned && !signed) {
throw new Error(
'Invalid type specified. Type must be either an unsigned integer (uint) or a signed integer (int).'
);
}

const bits = parseInt(type.slice(unsigned ? 4 : 3));

if (isNaN(bits) || bits % 8 !== 0 || bits < 8 || bits > 256) {
throw new Error(
'Invalid integer type specified. Bit size must be a multiple of 8 with a max of 256'
);
}

const number = BigNumber.from(value);
// range checks
if (unsigned) {
const max = BigNumber.from(2).pow(bits).sub(1);
return number.gte(0) && number.lte(max);
} else {
const halfRange = BigNumber.from(2).pow(bits - 1);
const min = halfRange.mul(-1);
const max = halfRange.sub(1);
return number.gte(min) && number.lte(max);
}
}
Copy link
Author

Choose a reason for hiding this comment

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

Validation for Integers.

  • check if number
  • check number is within range

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.

will spend more time on this monday

src/plugins/oSnap/utils/validators.ts Outdated Show resolved Hide resolved
@daywiss daywiss self-requested a review April 15, 2024 14:16
@gsteenkamp89 gsteenkamp89 changed the base branch from master to gerhard/uma-2547-adding-unsupported-treasury-crashes-proposal-page April 19, 2024 10:59
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.

looks good. this is not a blocker, but it would be great if you could show the tuple type in the field:
image
requestSettings (tuple[bool,bool,bool,intint])

i know you mentioned there was an issue with doing this so its ok the way it is if its a big lift

github-actions bot and others added 22 commits April 29, 2024 12:32
* fix: Privacy label name for any option

* lower case
* fix: Add created_gt filter to aliases query

* Use spread operator
* feat: Boost reward to winning option

* Change text
* feat: add `network` params to follow/unfollow sign

* fix: use different default network depending on env
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.

7 participants