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: transfer ownership #484

Merged
merged 7 commits into from
May 10, 2024
Merged

feat: transfer ownership #484

merged 7 commits into from
May 10, 2024

Conversation

VGabriel45
Copy link
Collaborator

@VGabriel45 VGabriel45 commented Apr 30, 2024

Added transferOwnership() method in BiconomySmartAccountV2.

When transferring ownership of a smart account using this method, it is important to specify the accountAddress parameter when logging in with the new owner. Failing to do so would result in the SDK utilizing the counterfactual address contract instead of the intended account address.

const _smartAccount = await createSmartAccountClient({ signer: walletClient, paymasterUrl, bundlerUrl, accountAddress: "0x...", })


PR-Codex overview

This PR introduces a new method transferOwnership to transfer ownership of a smart account. It also adds ECDSAModule ABI and related tests.

Detailed summary

  • Added transferOwnership method to transfer ownership of smart account
  • Included ECDSAModule ABI
  • Updated tests for ownership transfer and related functionality

The following files were skipped due to too many changes: tests/modules/write.test.ts

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Copy link

github-actions bot commented Apr 30, 2024

size-limit report 📦

Path Size
core (esm) 52.36 KB (+0.21% 🔺)
core (cjs) 56.95 KB (+0.14% 🔺)
account (tree-shaking) 51.32 KB (+0.03% 🔺)
bundler (tree-shaking) 2.33 KB (0%)
paymaster (tree-shaking) 2.27 KB (0%)
modules (tree-shaking) 40.05 KB (0%)

@livingrockrises
Copy link
Contributor

does the test cover connecting with new eoa once transfer ownership is mined?

@livingrockrises livingrockrises self-requested a review May 2, 2024 09:23
@VGabriel45
Copy link
Collaborator Author

does the test cover connecting with new eoa once transfer ownership is mined?

Yes

@VGabriel45 VGabriel45 changed the title Feat/transfer ownership feat: transfer ownership May 10, 2024
Copy link
Collaborator

@joepegler joepegler left a comment

Choose a reason for hiding this comment

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

Looks great. Couple of small change requests

.env.example Outdated Show resolved Hide resolved
src/account/BiconomySmartAccountV2.ts Outdated Show resolved Hide resolved
src/account/BiconomySmartAccountV2.ts Outdated Show resolved Hide resolved
tests/account/write.test.ts Show resolved Hide resolved
tests/modules/write.test.ts Outdated Show resolved Hide resolved
@joepegler
Copy link
Collaborator

Nice job

@joepegler joepegler merged commit 000fa86 into develop May 10, 2024
5 of 6 checks passed
@joepegler joepegler deleted the feat/transfer_ownership branch May 10, 2024 12:02
VGabriel45 added a commit that referenced this pull request May 13, 2024
* feat: gas offset param for sendTransaction and sendUserOp (#474)

* feat - gas offset param for sendTransaction

* added percentage based gas offsets

* added dummyPndOverride

* refactor gas offset percentage

* refactor names and ts doc + lint

* improved tsdoc & fixed comments

* refactor sendTransaction example tsdoc comment

* refactor sendTransaction test

---------

Co-authored-by: GabiDev <gv@popoo.io>

* feat: transfer ownership (#484)

feat: transfer ownership (#484)

* feat: transfer_ownership_v2 (#488)

* feat/transfer_ownership(in progress)

* cleanup

* fix linting

* added test for transferOwnership with session key manager module

* Fix linting

* refactor: refactor based on PR review

* improve ts doc + refactor tests

* added "moduleAddress" param to transferOwnership()

* fix module tests

* removed console.logs

* fixed lint + removed unused import

* remove unused import

* added argument type for module address

---------

Co-authored-by: GabiDev <gv@popoo.io>

---------

Co-authored-by: GabiDev <gv@popoo.io>
VGabriel45 added a commit that referenced this pull request May 14, 2024
* feat: gas offset param for sendTransaction and sendUserOp (#474)

* feat - gas offset param for sendTransaction

* added percentage based gas offsets

* added dummyPndOverride

* refactor gas offset percentage

* refactor names and ts doc + lint

* improved tsdoc & fixed comments

* refactor sendTransaction example tsdoc comment

* refactor sendTransaction test

---------

Co-authored-by: GabiDev <gv@popoo.io>

* feat: transfer ownership (#484)

feat: transfer ownership (#484)

* feat: transfer_ownership_v2 (#488)

* feat/transfer_ownership(in progress)

* cleanup

* fix linting

* added test for transferOwnership with session key manager module

* Fix linting

* refactor: refactor based on PR review

* improve ts doc + refactor tests

* added "moduleAddress" param to transferOwnership()

* fix module tests

* removed console.logs

* fixed lint + removed unused import

* remove unused import

* added argument type for module address

---------

Co-authored-by: GabiDev <gv@popoo.io>

* chore: release v4.3.0 (#491)

* release v4.3.0

* refactor: increase timeout for transferOwnership tests

---------

Co-authored-by: GabiDev <gv@popoo.io>

---------

Co-authored-by: GabiDev <gv@popoo.io>
Co-authored-by: Joe Pegler <joepegler123@gmail.com>
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.

4 participants