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

Feat extended safe info #580

Merged
merged 4 commits into from
Jun 12, 2024
Merged

Feat extended safe info #580

merged 4 commits into from
Jun 12, 2024

Conversation

compojoom
Copy link
Contributor

While working on this cowswap PR: cowprotocol/cowswap#4508 I discovered that the inconsistency in the responses of the sdk's getInfo and api-kit's getSafeInfo makes my life miserable.

I'm proposing to change the return type of the getInfo endpoint and align it a bit more to the SafeInfoResponse:
https://github.com/safe-global/safe-core-sdk/blob/8896940929eb23b154e0e6615226f1f7c004c7fb/packages/api-kit/src/types/safeTransactionServiceTypes.ts#L34

  readonly address: string
  readonly nonce: number
  readonly threshold: number
  readonly owners: string[]
  readonly singleton: string
  readonly modules: string[]
  readonly fallbackHandler: string
  readonly guard: string
  readonly version: string

Basically the only additional info to that is the chainId and isReadOnly flag.

@compojoom compojoom requested a review from katspaugh June 12, 2024 09:59
Copy link

changeset-bot bot commented Jun 12, 2024

🦋 Changeset detected

Latest commit: 60e8ed1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@safe-global/safe-apps-sdk Minor
@safe-global/safe-apps-web3modal Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Jun 12, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@compojoom
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@compojoom compojoom force-pushed the feat-extended-safe-info branch 2 times, most recently from d4af725 to c4c2718 Compare June 12, 2024 10:10
api-kit’s getSafeInfo endpoint returns way more information than our
communicator’s getInfo endpoint. This causes some dapps to use the
getInfo endpoint for just the readOnly flag (when running in an iframe)
and then to do api-kit getSafeInfo for the nonce and other props.
By returning nearly the same information from the getInfo endpoint we
make developers live easier.

.
@compojoom compojoom force-pushed the feat-extended-safe-info branch from c4c2718 to 5e61087 Compare June 12, 2024 10:14
@compojoom compojoom merged commit a811ac6 into main Jun 12, 2024
3 checks passed
@compojoom compojoom deleted the feat-extended-safe-info branch June 12, 2024 10:48
@github-actions github-actions bot locked and limited conversation to collaborators Jun 12, 2024
@dasanra dasanra restored the feat-extended-safe-info branch June 12, 2024 13:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants