-
Notifications
You must be signed in to change notification settings - Fork 12
feat: ✨ add a file deletion RPC for BSPs to stop storing a file #663
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
Open
TDemeco
wants to merge
20
commits into
main
Choose a base branch
from
feat/file-deletion-rpc
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.
Conversation
This file contains hidden or 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
This is useful both for debugging and also for providers that want to sign off (since they'll have to stop storing each file). This will be made more efficient in the future but is provided for now as the default method for sign off
…Labs/storage-hub into feat/file-deletion-rpc
This way we can avoid sending a stop storing confirmation if the pending stop storing request is no longer on chain
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
B5-clientnoteworthy
Changes should be mentioned client-related release notes
B7-runtimenoteworthy
Changes should be noted in any runtime-upgrade release notes
breaking
Needs to be mentioned in breaking changes
D4-nicetohaveaudit⚠️
PR contains trivial changes to logic that should be properly reviewed.
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.
We received a more-than-valid concern about the process that a BSP has to take to stop storing a certain file being too complicated to do by hand, since it requires crafting several proofs and waiting for a certain amount of blocks in the middle of it all, so in this PR we:
bspStopStoringFilethat allows BSP runners to easily initiate and complete the process to stop storing a certain file key, be it because they want to or they have been required to.RequestBspStopStoringRequestin the Blockchain Service's persistent state. The request is then processed when the forest root write lock becomes available, ensuring atomic proof generation and submission. We were not able to emit Blockchain Service commands from the RPC because of a circular dependency that we also solve in this PR, more info below.ProcessBspRequestStopStoringBlockchain Service event which is emitted when the forest root write lock is available and a request is pending. This handler is the one in charge of getting the file's metadata and generating the required inclusion proof to then be able to call thebsp_request_stop_storingextrinsic.ProcessBspConfirmStopStoringevent, which is emitted when the on-chain eventBspRequestedToStopStoringis detected and the minimum wait period has passed. Before submitting the confirm, the handler checks on-chain that the pending stop storing request still exists (to handle reorgs or manual confirmations). It then generates a new inclusion proof (since the BSP's forest could have changed in the meantime) and sends thebsp_confirm_stop_storingextrinsic.ForestProofVerificationFailed,FailedToApplyDeltafrom fisherman mutations), the request is automatically requeued for retry with a maximum of 3 attempts.peek_frontmethod to check if the first item's tick has been reached without modifying the queue, since items are chronologically ordered.ForestLockGuard) is used to automatically release the forest root write lock when the handler completes, regardless of how it returns (success, error, or panic).query_min_wait_for_stop_storing: Query the minimum ticks that a BSP has to wait between requesting to stop storing a file and being able to confirm to stop storing it.has_pending_stop_storing_request: Check if a pending stop storing request exists for a BSP and file key. This is used to verify the request still exists before submitting the confirm extrinsic.QueryMinWaitForStopStoringandHasPendingStopStoringRequest.getAllStoredFileKeysas a helper for a BSP runner that wants to obtain the list of file keys it is currently storing. This plus thebspStopStoringFileRPC method will be the way for a BSP to sign off for now until we design a simpler mechanism for the BSP to stop storing all its files.RpcHandlersused by the Blockchain Service to send extrinsics aArc<RwLock<Option<Arc<RpcHandlers>>>>instead of aArc<RpcHandlers>. This allows us to not have to set them when creating the Blockchain Service, and we can set them afterwards by using theSetRpcHandlerscommand.pub blockchain: Option<ActorHandle<BlockchainService<FSH, Runtime>>>field. This field is optional as for example the fisherman actor spawns the RPC service but does not have a Blockchain Service running.SetRpcHandlersis called, preventing "RPC handlers not yet available" errors during initialization.Short description
There are two new runtime APIs under the File System pallet:
query_min_wait_for_stop_storingandhas_pending_stop_storing_request. Runtime managers of runtimes that use StorageHub will have to implement them.There has also been a change in the parameters received by
init_sh_builderso any node managers that use it will have to be updated.Finally, there's new columns in the RocksDB permanent storage of the node.
Who is affected
init_sh_builderandfinish_sh_builder_and_run_tasksSuggested code changes
Implement the runtime APIs:
impl pallet_file_system_runtime_api::FileSystemApi<Block, AccountId, BackupStorageProviderId, MainStorageProviderId, H256, BlockNumber, ChunkId, BucketId, StorageRequestMetadata, BucketId, StorageDataUnit, H256> for Runtime {
...
fn query_min_wait_for_stop_storing() -> BlockNumber {
FileSystem::query_min_wait_for_stop_storing()
}
}
Add the new parameters to
init_sh_builder: