This repository has been archived by the owner on Feb 1, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 32
delete by prefix rfc #39
Open
yoni-wolf
wants to merge
5
commits into
hyperledger-archives:main
Choose a base branch
from
yoni-wolf:delete-prefix
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,135 @@ | ||
- Feature Name: delete_by_prefix | ||
- Start Date: 2019-2-20 | ||
- RFC PR: | ||
- Sawtooth Issue: | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
This RFC proposes an additional API to transaction context, | ||
context.deletePrefix(address_prefix) which will get an address prefix as input | ||
parameter and delete all the addresses in the prefix subtree. | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
The motivation came when looking for an efficient way to delete chunk of | ||
addresses, for example all addresses that are allocated for a certain user. | ||
If each member has some unique address prefix, deleting all the addresses | ||
belong to a member can become more efficient with delete by prefix. | ||
|
||
Since addresses are stored in a radix tree, deleting many addresses by one | ||
prefix is much faster than deleting the same amount of addresses one by one. | ||
Furthermore, delete by prefix doesn't require transaction processor to know | ||
which addresses exist. This will remove the need to store huge list of | ||
existing addresses in a special address that will need to be modified by every | ||
transaction that writes a new address. | ||
|
||
# Guide-level explanation | ||
[guide-level-explanation]: #guide-level-explanation | ||
|
||
A context API to delete address already exist for transaction processors. | ||
The existing delete API accepts only full 35 bytes addresses. | ||
This RFC exposes to transaction processor a new capability of deleting by | ||
address prefix. When delete by prefix is called, all addresses under the | ||
prefix subtree will be deleted. | ||
|
||
Main usage for this feature is the need to support some 'clean transaction' | ||
that will remove all addresses with some communality, for example all | ||
addresses that belong to some member. | ||
The proposed solution is to have address mapping logic that allocates some | ||
unique pre-defined address prefix per member. The transaction processor logic | ||
will know to calculate the address suffix based on transaction payload data | ||
and execute any transaction. | ||
In order to execute the clean transaction, the transaction processor will | ||
call context.deletePrefix with the member prefix. | ||
|
||
The existing solution is to keep track of all used addresses, this will | ||
require read-modify-write of a special address for every new address that is | ||
used. This solution could work but will slow down executing of every regular | ||
transactions while the clean transaction is rare. | ||
|
||
No change is recommended to the existing methods, so the implementation of this | ||
RFC should not cause any breakage for pre-existing transaction processors. | ||
|
||
# Reference-level explanation | ||
[reference-level-explanation]: #reference-level-explanation | ||
|
||
Add new API to context: deletePrefix that delete a subtree of addresses | ||
starting from a given address prefix. | ||
This API should be similar to regular delete address API but can get as input | ||
prefix of address and not just full 35 bytes address. | ||
Calling this API with full address should behave exactly like calling existing | ||
delete API. | ||
|
||
In the suggested implementation there are two proto changes: | ||
|
||
Addition to state_context.proto: | ||
|
||
// A request from the handler/tp to delete state prefix entries at an | ||
// collection of addresses prefix subtree | ||
message TpStateDeletePrefixRequest { | ||
string context_id = 1; | ||
repeated string addresses_prefix = 2; | ||
} | ||
|
||
// A response from the context_manager/validator with the addresses | ||
// that were deleted | ||
message TpStateDeleteResponse { | ||
yoni-wolf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
enum Status { | ||
STATUS_UNSET = 0; | ||
OK = 1; | ||
AUTHORIZATION_ERROR = 2; | ||
} | ||
|
||
Status status = 1; | ||
} | ||
|
||
Addition to validator.proto: | ||
|
||
// State delete prefix request from the transaction processor to the | ||
// validator/context_manager | ||
TP_STATE_DELETE_PREFIX_REQUEST = 17; | ||
// State delete prefix response from the validator/context_manager to the | ||
// transaction processor | ||
TP_STATE_DELETE_PREFIX_RESPONSE = 18; | ||
|
||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
1. This will need to be implemented in every sawtooth SDK. | ||
2. Once implemented it will be difficult to deprecate because of our | ||
commitment to backward compatibility. | ||
3. This requires transaction processor developers to understand the risk of | ||
calling delete by prefix and accidently deleting unintended addresses. | ||
Delete by prefix should be used only when developer is sure prefix is | ||
unique like when having a pre-defined prefix and not in case there could | ||
be hash collisions for the prefix. | ||
|
||
# Rationale and alternatives | ||
[alternatives]: #alternatives | ||
|
||
An alternative could be in the transaction processor logic, store an address | ||
that contains list of existing addresses. When need to delete multiple | ||
addresses transaction processor will first read the special address, then | ||
delete each address in the list. | ||
This has a performance downside that each time transaction processor needs to | ||
write a new address, it will need to modify the special address containing | ||
list of all addresses. This address could be huge and it needs to be changed | ||
for almost every transaction, while the need to delete all the addresses in | ||
the list is very rare. | ||
|
||
# Prior art | ||
[prior-art]: #prior-art | ||
|
||
Access address by prefix is a key capability of radix tree. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is worded badly. In fact, I don't know that I would go so far as to even include prior art, since it is an extension of current Sawtooth - the Merkle-Radix trie - which that functionality made use of prior art. |
||
This RFC intends to take advantage of sawtooth design to use a radix tree for | ||
storing the global state. | ||
|
||
# Unresolved questions | ||
[unresolved]: #unresolved-questions | ||
|
||
Implementation of the validator changes for the new context API are not | ||
prescribed in this RFC. | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we extend existing API and allow partial address?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could.
I thought in order to prevent TP developers from deleting prefix by mistake, the right approach will be to add a new API for this instead of adding capabilities to existing delete API.
Do you think it will be better to allow delete with prefix by extending existing API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this can however be controlled by outputs field in TransactionHeader.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would extending the existing API look like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yoni-wolf please correct me, but I think that would look like the existing state delete call would take a list of address prefixes rather than a list of addresses. The validator would then need to distinguish each element of the list as between a full 70 character address as a single address or something less than 70 as a prefix.
Combining this: https://github.com/hyperledger/sawtooth-core/blob/master/protos/state_context.proto#L66-L70
With this: https://github.com/hyperledger/sawtooth-core/blob/master/protos/state_context.proto#L66-L70
@vaporos if that's not what you were asking please elaborate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarification was that this option should be included in the alternatives section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added to the alternative section the option to use existing API instead of adding a new API