-
Notifications
You must be signed in to change notification settings - Fork 165
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
Add IsWorkerAddress method #756
Changes from 6 commits
3b27c29
c6ef699
aace3de
aa4cdea
dafba87
97c5a06
683b8f1
fdd3cbe
86a66a4
4b0f149
2acfc9b
a8dc6e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
--- | ||
fip: "0066" | ||
title: Extension of Runtime Methods for Address Verification | ||
author: "David Casem <@dcasem>" | ||
discussions-to: https://github.com/filecoin-project/FIPs/discussions/401 | ||
status: Draft | ||
type: Technical | ||
category: Core | ||
created: 2023-07-24 | ||
--- | ||
|
||
## Simple Summary | ||
Introduces a method to the built-in Miner actor for address validation to be used in determining \ | ||
whether a given address corresponds to the worker of the queried Miner on the Filecoin network. | ||
|
||
## Abstract | ||
The proposed FIP introduces a new method to the built-in miner actor for address validation to be used in determining \ | ||
whether a given address corresponds to the worker of the queried Miner on the Filecoin network. | ||
|
||
## Change Motivation | ||
There are several scenarios in the Filecoin protocol where it is necessary to confirm if a particular address belongs to a miner actor as a worker key. Having a dedicated exported method in the miner actorto handle such checks can increase efficiency and readability of the codebase. | ||
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. (Editor comment) This is a weak motivation without examples. Can you please list 3-4 such examples? 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. The primary example is to facilitate associating a miner with a voter, given that many SPs hand over their owner wallet keys to a lender. 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. |
||
|
||
## Specification | ||
We propose to introduce a new method: `is_worker_address()` to the Miner actor. This function receives an address as parameter and returns a boolean indicating whether this address corresponds to a worker in the network or not. The function implements a logic to resolve the address and checks it against the current worker address. | ||
|
||
Pseudocode | ||
|
||
The pseudocode below demonstrates the intended implementation of the `is_worker_address()` function: | ||
```function is_worker_address(rt: Runtime, params: IsWorkerAddressParams) returns Result<IsWorkerAddressReturn, ActorError>: | ||
|
||
// Accept any caller | ||
rt.validate_immediate_caller_accept_any() | ||
|
||
// Resolve the address | ||
address = rt.resolve_address(params.address) | ||
|
||
// If address is null, return false | ||
if address is null: | ||
return Ok(IsWorkerAddressReturn { is_worker: false }) | ||
|
||
// Fetch the current state | ||
state = rt.state() | ||
|
||
// Retrieve the miner info | ||
info = get_miner_info(rt.store(), state) | ||
|
||
// Check if the resolved address matches the worker address | ||
is_worker = (address == info.worker) | ||
|
||
// Return the result | ||
return Ok(IsWorkerReturn { is_worker }) | ||
``` | ||
|
||
## Backwards Compatibility | ||
This proposal should not have any backward compatibility issues. The function is a new addition and does not alter any existing methods or structures. | ||
|
||
## Test Cases | ||
To ensure the correctness of the `is_worker_address()` function, multiple test cases covering different scenarios (worker/non-worker addresses, non-resolvable addresses etc.) should be considered | ||
|
||
|
||
Possible Tests: | ||
|
||
`IsWorkerAddress` correctly identifies the worker address | ||
|
||
`IsWorkerAddress` correctly identifies non-worker addresses | ||
Comment on lines
+63
to
+65
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. (Peer review) Good that we're thinking about positive and negative testing, but these definitions are a bit unspecific. Consider test cases involving miners that have no worker, and queries with different address types. |
||
|
||
## Implementation | ||
This FIP does not require any changes to existing protocols. The changes would be implemented in the actor package in the. The exact implementation details will depend on the codebase at the time of implementation. | ||
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. (Peer) Introducing any change in Wasm bytecode is protocol-breaking because bytecode is referenced by CID. |
||
|
||
|
||
## Security Considerations | ||
The proposed function does not introduce any new security considerations as it only checks existing data structures. It is important to ensure that the function is implemented correctly to avoid any potential security issues. | ||
|
||
## Implementation | ||
This FIP does not require any changes to existing protocols. The changes would be implemented in the `actor` package. The exact implementation details will depend on the codebase at the time of implementation. | ||
|
||
## Copyright | ||
Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/). |
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.
Leave as FIP00XX for now. There is already a FIP0066. FIP Editors assign FIP numbers once the draft is reviewed and merged.