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

fix(vats): don't give all of bankManager to 1 account #9354

Merged
merged 1 commit into from
May 10, 2024

Conversation

dckc
Copy link
Member

@dckc dckc commented May 9, 2024

refs: #9342, #9193
stacked on

Description

Provide localchain accounts only with the bank for the relevant address, rather than giving them access to all accounts in the form of the bankManager.

earlier discussion: https://github.com/Agoric/agoric-sdk/pull/9342/files#r1594791187

Security Considerations

bankManager was excess authority

Scaling / Documentation / Testing Considerations

Nothing significant: no scaling changes; existing docs/tests suffice.

Upgrade Considerations

code is not released / deployed

Copy link

cloudflare-workers-and-pages bot commented May 9, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: d1f1630
Status: ✅  Deploy successful!
Preview URL: https://d0c44faf.agoric-sdk.pages.dev
Branch Preview URL: https://dc-localchain-1.agoric-sdk.pages.dev

View logs

@dckc dckc force-pushed the dc-localchain-1 branch from b161841 to 42d4184 Compare May 9, 2024 16:52
@dckc dckc force-pushed the dc-powers-refactor branch from c38be56 to bef5e84 Compare May 9, 2024 17:33
@dckc dckc force-pushed the dc-localchain-1 branch from 871272a to 38a3d77 Compare May 9, 2024 17:38
@dckc dckc marked this pull request as ready for review May 9, 2024 17:55
@dckc dckc requested review from turadg, michaelfig and 0xpatrickdev May 9, 2024 17:55
Copy link
Member

@turadg turadg left a comment

Choose a reason for hiding this comment

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

👍

@dckc dckc force-pushed the dc-localchain-1 branch from 38a3d77 to e04770a Compare May 9, 2024 18:33
@dckc dckc force-pushed the dc-powers-refactor branch from a59a230 to 4433526 Compare May 9, 2024 21:17
@dckc dckc force-pushed the dc-localchain-1 branch from e04770a to fdcaa30 Compare May 9, 2024 21:19
Base automatically changed from dc-powers-refactor to master May 9, 2024 22:19
mergify bot added a commit that referenced this pull request May 9, 2024
follow-up to: #9086

## Description

In preparation for addressing excess authority (#9354):

- To allow for state upgrade, rather than introducing the complexity of
the power store in this initial version, reserve one property for future
use
- Require all localChain powers at intialization, which eliminates the
need for the admin.setPower method.

### Security / Documentation Considerations

n/a

### Scaling Considerations

small constant factor change: removing the indirection thru the power
store removes 1 syscall per access

### Testing Considerations

existing tests suffice

### Upgrade Considerations

code is not yet released / deployed
@dckc dckc added the automerge:rebase Automatically rebase updates, then merge label May 9, 2024
@dckc dckc force-pushed the dc-localchain-1 branch from fdcaa30 to d1f1630 Compare May 9, 2024 23:07
@mergify mergify bot merged commit 4685ea9 into master May 10, 2024
65 checks passed
@mergify mergify bot deleted the dc-localchain-1 branch May 10, 2024 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants