-
Notifications
You must be signed in to change notification settings - Fork 582
feat: allow starknet address in schema and when signing #1152
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: allow starknet address in schema and when signing #1152
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request updates the schema to support both EVM and Starknet addresses for members, admins, and moderators.
- Updated the "members" property by replacing the single format with an anyOf clause that allows either EVM or Starknet addresses.
- Applied similar changes to the "admins" and "moderators" properties.
3a853c5
to
334a89b
Compare
334a89b
to
e0131fe
Compare
…s://github.com/snapshot-labs/snapshot.js into feat-accept-starknet-address-in-space-settings
export const proposalTypes = { | ||
Proposal: [ | ||
{ name: 'from', type: 'address' }, | ||
{ name: 'from', type: 'string' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure Starknet address doesn't work as address
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using the address
type return the following error:
Error: cannot resolve ENS names without a provider (operation="resolveName", value="", code=UNSUPPORTED_OPERATION, version=wallet/5.8.0)
at Logger.makeError (/Users/wan/Documents/GitHub/snapshot.js-bootstrap/node_modules/@ethersproject/logger/lib/index.js:238:21)
at Logger.throwError (/Users/wan/Documents/GitHub/snapshot.js-bootstrap/node_modules/@ethersproject/logger/lib/index.js:247:20)
at /Users/wan/Documents/GitHub/snapshot.js-bootstrap/node_modules/@ethersproject/wallet/lib/index.js:182:40
at Function.<anonymous> (/Users/wan/Documents/GitHub/snapshot.js-bootstrap/node_modules/@ethersproject/hash/lib/typed-data.js:422:46)
at step (/Users/wan/Documents/GitHub/snapshot.js-bootstrap/node_modules/@ethersproject/hash/lib/typed-data.js:33:23)
at Object.next (/Users/wan/Documents/GitHub/snapshot.js-bootstrap/node_modules/@ethersproject/hash/lib/typed-data.js:14:53)
at /Users/wan/Documents/GitHub/snapshot.js-bootstrap/node_modules/@ethersproject/hash/lib/typed-data.js:8:71
at new Promise (<anonymous>)
at __awaiter (/Users/wan/Documents/GitHub/snapshot.js-bootstrap/node_modules/@ethersproject/hash/lib/typed-data.js:4:12)
at TypedDataEncoder.resolveNames (/Users/wan/Documents/GitHub/snapshot.js-bootstrap/node_modules/@ethersproject/hash/lib/typed-data.js:392:16) {
reason: 'cannot resolve ENS names without a provider',
code: 'UNSUPPORTED_OPERATION',
operation: 'resolveName',
value: ''
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it correct to say you are adding support for from
as string
but from
as address
is still available? We should not use string
for Ethereum addresses ideally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it correct to say you are adding support for
from
asstring
butfrom
asaddress
is still available? We should not usestring
for Ethereum addresses ideally.
It will be always as string, from the sdk side. We validate the from
with a getAddress
before signing, so this field will always be a valid address when submitted.
People signing themselves, without the sdk and passing the types manually can continue signing using address
"dd19ee4357e5bc813529e1b537b77ccb767135701f0223434f3b53f1ac03dcc6": "delete-proposal", | ||
"817265460009cfdbde0752fcc7ef629ae7e05a6e8b6cbffab8a7cbf6cd19e87e": "delete-proposal", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two new delete-proposal 🤔 i think only one should be added for current change
same for vote vote-string and vote-array, there are two added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed the duplicate one for proposal, vote-
are there twice because of the Vote2
type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact no, duplicate proposals type are also legit, due to them being twice in the types.ts (Proposal2
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should remove one, first one can be deprecated i think , but maybe in a different PR
…s://github.com/snapshot-labs/snapshot.js into feat-accept-starknet-address-in-space-settings
This reverts commit c83fa2a.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utAck
Toward https://github.com/snapshot-labs/workflow/issues/562
This PR updates the schema and signing logic to accept and sign a payload submitted by starknet address alias
from
field of the payloadproposal
,delete-proposal
andvote-
to havefrom
field asstring
instead ofaddress
Test
from
field.proposal
orvote
function with the corresponding payloadUNSUPPORTED_OPERATION
error