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

Add IsWorkerAddress method #756

Closed
wants to merge 12 commits into from
78 changes: 78 additions & 0 deletions FIPS/fip-0066.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
---
fip: "0066"
Copy link
Collaborator

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.

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.
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with @raulk.

@dcasem can you please commit changes directly to your FIP draft?


## 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
Copy link
Member

Choose a reason for hiding this comment

The 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.
Copy link
Member

Choose a reason for hiding this comment

The 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/).