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(wallet-ui): reenable solo-only wallet #6498

Merged
merged 2 commits into from
Oct 26, 2022
Merged

feat(wallet-ui): reenable solo-only wallet #6498

merged 2 commits into from
Oct 26, 2022

Conversation

michaelfig
Copy link
Member

refs: #6497

Description

Put the built (solo-only) wallet from the community-dev branch where the ag-solo web server can find it.

Security Considerations

Documentation Considerations

Testing Considerations

@michaelfig michaelfig requested a review from dckc October 25, 2022 20:53
@michaelfig michaelfig self-assigned this Oct 25, 2022
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

It's not clear to me how maintenance of this is intended to work. I suspect there's some rationale for this approach that we would benefit from writing down explicitly.

  • build products in the source tree
  • old BLD icons
  • another copy of lockdown

Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

wow... much cleaner... but now I don't really understand how it works. clue me in?

@michaelfig
Copy link
Member Author

wow... much cleaner... but now I don't really understand how it works. clue me in?

Here is a draft of what I intend to put in packages/wallet/README.md. LMK if it's clear enough, and then I'll add it to this PR.


Agoric Wallet

This directory contains two Agoric Wallet pieces:

  • api - the legacy "off-chain" wallet used by @agoric/solo
  • ui - the @agoric/smart-wallet user interface

The smart wallet has not yet subsumed all the features of the off-chain wallet. Until it does, we require the api directory.

The ui directory no longer supports the off-chain wallet. In order to work around this, we point the solo web server at a wallet UI package that does.

Upgrading the solo wallet UI

Change the legacy wallet UI:

  1. Check out the Agoric SDK branch containing a working ag-solo wallet (such as community-dev).
  2. yarn && yarn build
  3. cd packages/wallet/ui
  4. Develop any changes to the wallet, then yarn build
  5. yarn publish, setting the version number to end in -solo.N where N is greater than the previous.
  6. Commit, create a PR and target it at the same branch in step 1.

Next, you can update the master branch:

  1. Check out the Agoric SDK master branch.
  2. Set the version of @agoric/wallet-ui in the packages/wallet/package.json to be the one you published (the exact version, no caret or tilde).
  3. yarn
  4. Test your wallet updates by running an ag-solo (sim chain is fine).
  5. Commit, create a PR and target it at master.

After merging the first PR, you can merge the second.

Copy link

@godwn2890 godwn2890 left a comment

Choose a reason for hiding this comment

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

Copy link

@godwn2890 godwn2890 left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

the approach in the README comment looks good!

@michaelfig michaelfig added the automerge:no-update (expert!) Automatically merge without updates label Oct 26, 2022
@mergify mergify bot merged commit 450e937 into master Oct 26, 2022
@mergify mergify bot deleted the mfig-solo-wallet branch October 26, 2022 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:no-update (expert!) Automatically merge without updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants