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

localchain queryMany and bankManager #9086

Merged
merged 2 commits into from
Mar 14, 2024
Merged

Conversation

turadg
Copy link
Member

@turadg turadg commented Mar 13, 2024

Description

From #8624, localchain changes that don't depend on vtransfer

Security Considerations

Creates a PowerStore map that is mutable

Scaling Considerations

Storing the PowerStore.

Documentation Considerations

Some new APIs. No separate docs exist yet to update.

Testing Considerations

queryMany is covered in tests.

Upgrade Considerations

Not yet deployed

@turadg turadg requested a review from michaelfig March 13, 2024 22:17
@turadg turadg added the automerge:rebase Automatically rebase updates, then merge label Mar 13, 2024
Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

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

Thanks for this!

* Deposit a payment into the bank purse that matches the alleged brand.
* This is safe, since even if the payment lies about its brand, ERTP will
* reject spoofed payment objects when depositing into a purse.
*
Copy link
Member

Choose a reason for hiding this comment

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

I don't expect this change for this PR, but I think we should have a solid story around ERTP payment recovery sets and how to use them to prevent orphaned payments if the deposit or getAllegedBrand methods get stuck.

@turadg turadg force-pushed the ta/localchain-queryMany branch from af2a2cb to 5b789c5 Compare March 14, 2024 05:19
@mergify mergify bot merged commit accf269 into master Mar 14, 2024
66 checks passed
@mergify mergify bot deleted the ta/localchain-queryMany branch March 14, 2024 05:54
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
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.

2 participants