Skip to content

Commit

Permalink
refactor: types/address -> address/address
Browse files Browse the repository at this point in the history
- `types.Address` -> `address.Address`
- `types.AddrSet` -> `address.Set`
- `types.NewAddress()` -> `address.New()` (and same for similarly named methods)

Ref #830
  • Loading branch information
dignifiedquire authored Aug 31, 2018
1 parent 5df62c0 commit efdc601
Show file tree
Hide file tree
Showing 98 changed files with 1,014 additions and 957 deletions.
22 changes: 10 additions & 12 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ code walk-through if it would be helpful.
## Code Reviews

With "prioritize progress" as a primary directive we can derive some
corollaries for code reviews:
corollaries for code reviews:

* Unless a reviewee asks for it, **avoid lengthy design discussions
in PR reivews**. Design discussions in PRs shouldn't consume a lot
Expand All @@ -54,9 +54,9 @@ corollaries for code reviews:
Exercise judgement about when not to defer design discussion. For
example if deferring would ultimately require lots of painful
refactoring or undoing lots of work, consider not deferring.

* Limit scope of comments to the story itself: avoid feature creep.
As above, prefer to defer the addition of new features to followup
As above, prefer to defer the addition of new features to followup
work.

* Get comfortable with "good enough" and recognize that "good
Expand All @@ -79,7 +79,7 @@ the following protocol:
should consider them but doesn't _have_ to respond or
address them
* a comment that says "BLOCKING" must be addressed
and responded to. A reviewer has to decide how
and responded to. A reviewer has to decide how
to deliver a blocking comment: via "Request Changes" (merge blocking) or via
"Add Comments" or "Approve" (not merge blocking):
* If a reviewer makes a blocking comment while blocking merge
Expand All @@ -93,13 +93,13 @@ the following protocol:
error. It's mandatory to fix but doesn't necessarily require
another look from the reviewer.
* Approval means approval even in the face of minor changes.
github should be configured to allow merging with earlier
github should be configured to allow merging with earlier
approval even after rebasing/minor changes.

**Do not just leave comments in a code review.** Comments should be
blocking or come with an approval unless you are still looking things
over or you're asking for clarification. It's ok/encouraged to ask
for explanations. The thing we want to avoid is *unnecessarily*
for explanations. The thing we want to avoid is *unnecessarily*
requiring mutiple round trips from someone whose next availability
might be 12 hours away.

Expand All @@ -119,7 +119,7 @@ requiring mutiple round trips from someone whose next availability
| Blocked | If you are working on something but get stuck because of external factors. |
| Closed | :tada:|

- The first and last stages are kept up-to-date automatically.
- The first and last stages are kept up-to-date automatically.
- New issues created in `go-filecoin` show up automatically in `Backlog`
- When a PR is merged, the referenced issues are closed and moved to `Closed`.
- All other stages are updated manually.
Expand Down Expand Up @@ -161,7 +161,7 @@ There are always exceptions but generally:
* Protocol messages are nouns (eg, `DealQuery`, `DealResponse`) and their handlers are verbs (eg, `QueryDeal`)
* Do not put implementation inline in command functions. Command implementation should be minimal, calling out functionality that exists elsewhere (eg on the node). Commands implementation is an important API which gets muddled when implementation happens inline in command functions.

We use the following import ordering.
We use the following import ordering.
```
import (
[stdlib packages, alpha-sorted]
Expand Down Expand Up @@ -226,7 +226,7 @@ for message processing. Features should be informed by an awareness of
related platforms.

## Testing Philosophy
* All code must be unit tested and should hit our target coverage rate (80%).
* All code must be unit tested and should hit our target coverage rate (80%).
* We prefer to test the output/contracts, not the individual lines of code (which we expect to change significantly during early work).
* Daemon tests (integration tests that run a node and send it commands):
* Daemon tests are not a substitute for unit tests: the foo command implementation should be unit tested in the `foo_test.go` file
Expand Down Expand Up @@ -280,6 +280,4 @@ Likely future requirements:
* Don't use `==` to compare `*types.Block`; use `Block.Equals()`
* Ditto for `*cid.Cid`
* For `types.Message` use `types.MsgCidsEqual` (or better submit a PR to add `Message.Equals()`)
* DO use `==` for `types.Address`, it's just an address


* DO use `==` for `address.Address`, it's just an address
17 changes: 9 additions & 8 deletions abi/abi.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"gx/ipfs/QmQsErDt8Qgw1XrsXf2BpEzDgGWtB1YLsTAARBup5b6B9W/go-libp2p-peer"
cbor "gx/ipfs/QmV6BQ6fFCf9eFHDuRxvguvqfKLZtZrxthgZvDfRCs4tMN/go-ipld-cbor"

"github.com/filecoin-project/go-filecoin/address"
"github.com/filecoin-project/go-filecoin/types"
)

Expand All @@ -20,7 +21,7 @@ type Type uint64
const (
// Invalid is the default value for 'Type' and represents an errorneously set type.
Invalid = Type(iota)
// Address is a types.Address
// Address is a address.Address
Address
// AttoFIL is a *types.AttoFIL
AttoFIL
Expand All @@ -47,7 +48,7 @@ func (t Type) String() string {
case Invalid:
return "<invalid>"
case Address:
return "types.Address"
return "address.Address"
case AttoFIL:
return "*types.AttoFIL"
case BytesAmount:
Expand Down Expand Up @@ -82,7 +83,7 @@ func (av *Value) String() string {
case Invalid:
return "<invalid>"
case Address:
return av.Val.(types.Address).String()
return av.Val.(address.Address).String()
case AttoFIL:
return av.Val.(*types.AttoFIL).String()
case BytesAmount:
Expand Down Expand Up @@ -121,9 +122,9 @@ func (av *Value) Serialize() ([]byte, error) {
case Invalid:
return nil, ErrInvalidType
case Address:
addr, ok := av.Val.(types.Address)
addr, ok := av.Val.(address.Address)
if !ok {
return nil, &typeError{types.Address{}, av.Val}
return nil, &typeError{address.Address{}, av.Val}
}
return addr.Bytes(), nil
case AttoFIL:
Expand Down Expand Up @@ -198,7 +199,7 @@ func ToValues(i []interface{}) ([]*Value, error) {
out := make([]*Value, 0, len(i))
for _, v := range i {
switch v := v.(type) {
case types.Address:
case address.Address:
out = append(out, &Value{Type: Address, Val: v})
case *types.AttoFIL:
out = append(out, &Value{Type: AttoFIL, Val: v})
Expand Down Expand Up @@ -244,7 +245,7 @@ func FromValues(vals []*Value) []interface{} {
func Deserialize(data []byte, t Type) (*Value, error) {
switch t {
case Address:
addr, err := types.NewAddressFromBytes(data)
addr, err := address.NewFromBytes(data)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -315,7 +316,7 @@ func Deserialize(data []byte, t Type) (*Value, error) {
}

var typeTable = map[Type]reflect.Type{
Address: reflect.TypeOf(types.Address{}),
Address: reflect.TypeOf(address.Address{}),
AttoFIL: reflect.TypeOf(&types.AttoFIL{}),
Bytes: reflect.TypeOf([]byte{}),
BytesAmount: reflect.TypeOf(&types.BytesAmount{}),
Expand Down
5 changes: 2 additions & 3 deletions abi/encoding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ import (
"math/big"
"testing"

"github.com/filecoin-project/go-filecoin/types"

"github.com/filecoin-project/go-filecoin/address"
"github.com/stretchr/testify/assert"
)

Expand All @@ -14,7 +13,7 @@ import (
// things.

func TestBasicEncodingRoundTrip(t *testing.T) {
addrGetter := types.NewAddressForTestGetter()
addrGetter := address.NewForTestGetter()

cases := map[string][]interface{}{
"empty": nil,
Expand Down
2 changes: 1 addition & 1 deletion actor/actor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func NewMockActor(list exec.Exports) *MockActor {
}

func makeCtx(method string) exec.VMContext {
addrGetter := types.NewAddressForTestGetter()
addrGetter := address.NewForTestGetter()
return vm.NewVMContext(nil, nil, types.NewMessage(addrGetter(), addrGetter(), 0, nil, method, nil), nil, nil, types.NewBlockHeight(0))
}

Expand Down
12 changes: 6 additions & 6 deletions actor/builtin/miner/miner.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type Actor struct{}

// State is the miner actors storage.
type State struct {
Owner types.Address
Owner address.Address

// PeerID references the libp2p identity that the miner is operating.
PeerID peer.ID
Expand Down Expand Up @@ -82,7 +82,7 @@ func NewActor() *actor.Actor {
}

// NewState creates a miner state struct
func NewState(owner types.Address, key []byte, pledge *types.BytesAmount, pid peer.ID, collateral *types.AttoFIL) *State {
func NewState(owner address.Address, key []byte, pledge *types.BytesAmount, pid peer.ID, collateral *types.AttoFIL) *State {
return &State{
Owner: owner,
PeerID: pid,
Expand Down Expand Up @@ -206,18 +206,18 @@ func (ma *Actor) AddAsk(ctx exec.VMContext, price *types.AttoFIL, size *types.By
}

// GetOwner returns the miners owner.
func (ma *Actor) GetOwner(ctx exec.VMContext) (types.Address, uint8, error) {
func (ma *Actor) GetOwner(ctx exec.VMContext) (address.Address, uint8, error) {
var state State
out, err := actor.WithState(ctx, &state, func() (interface{}, error) {
return state.Owner, nil
})
if err != nil {
return types.Address{}, errors.CodeError(err), err
return address.Address{}, errors.CodeError(err), err
}

a, ok := out.(types.Address)
a, ok := out.(address.Address)
if !ok {
return types.Address{}, 1, errors.NewFaultErrorf("expected an Address return value from call, but got %T instead", out)
return address.Address{}, 1, errors.NewFaultErrorf("expected an Address return value from call, but got %T instead", out)
}

return a, 0, nil
Expand Down
8 changes: 4 additions & 4 deletions actor/builtin/miner/miner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ import (
"github.com/stretchr/testify/require"
)

func createTestMiner(assert *assert.Assertions, st state.Tree, vms vm.StorageMap, minerOwnerAddr types.Address, key []byte, pid peer.ID) types.Address {
func createTestMiner(assert *assert.Assertions, st state.Tree, vms vm.StorageMap, minerOwnerAddr address.Address, key []byte, pid peer.ID) address.Address {
pdata := actor.MustConvertParams(types.NewBytesAmount(10000), key, pid)
nonce := core.MustGetNonce(st, address.TestAddress)
msg := types.NewMessage(minerOwnerAddr, address.StorageMarketAddress, nonce, types.NewAttoFILFromFIL(100), "createMiner", pdata)

result, err := core.ApplyMessage(context.Background(), st, vms, msg, types.NewBlockHeight(0))
assert.NoError(err)

addr, err := types.NewAddressFromBytes(result.Receipt.Return[0])
addr, err := address.NewFromBytes(result.Receipt.Return[0])
assert.NoError(err)
return addr
}
Expand Down Expand Up @@ -184,7 +184,7 @@ func TestPeerIdGetterAndSetter(t *testing.T) {
})
}

func updatePeerIdSuccess(ctx context.Context, t *testing.T, st state.Tree, vms vm.StorageMap, fromAddr types.Address, minerAddr types.Address, newPid peer.ID) {
func updatePeerIdSuccess(ctx context.Context, t *testing.T, st state.Tree, vms vm.StorageMap, fromAddr address.Address, minerAddr address.Address, newPid peer.ID) {
require := require.New(t)

updatePeerIdMsg := types.NewMessage(
Expand All @@ -201,7 +201,7 @@ func updatePeerIdSuccess(ctx context.Context, t *testing.T, st state.Tree, vms v
require.Equal(uint8(0), applyMsgResult.Receipt.ExitCode)
}

func getPeerIdSuccess(ctx context.Context, t *testing.T, st state.Tree, vms vm.StorageMap, fromAddr types.Address, minerAddr types.Address) peer.ID {
func getPeerIdSuccess(ctx context.Context, t *testing.T, st state.Tree, vms vm.StorageMap, fromAddr address.Address, minerAddr address.Address) peer.ID {
res, code, err := core.CallQueryMethod(ctx, st, vms, minerAddr, "getPeerID", []byte{}, fromAddr, nil)
require.NoError(t, err)
require.Equal(t, uint8(0), code)
Expand Down
23 changes: 12 additions & 11 deletions actor/builtin/paymentbroker/paymentbroker.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/filecoin-project/go-filecoin/abi"
"github.com/filecoin-project/go-filecoin/actor"
"github.com/filecoin-project/go-filecoin/address"
"github.com/filecoin-project/go-filecoin/exec"
"github.com/filecoin-project/go-filecoin/types"
"github.com/filecoin-project/go-filecoin/vm"
Expand Down Expand Up @@ -65,7 +66,7 @@ type accountPaymentChannels map[string]*PaymentChannel

// PaymentChannel records the intent to pay funds to a target account.
type PaymentChannel struct {
Target types.Address `json:"target"`
Target address.Address `json:"target"`
Amount *types.AttoFIL `json:"amount"`
AmountRedeemed *types.AttoFIL `json:"amount_redeemed"`
Eol *types.BlockHeight `json:"eol"`
Expand All @@ -74,8 +75,8 @@ type PaymentChannel struct {
// PaymentVoucher is a voucher for a payment channel that can be transferred off-chain but guarantees a future payment.
type PaymentVoucher struct {
Channel types.ChannelID `json:"channel"`
Payer types.Address `json:"payer"`
Target types.Address `json:"target"`
Payer address.Address `json:"payer"`
Target address.Address `json:"target"`
Amount types.AttoFIL `json:"amount"`
Signature Signature `json:"signature"`
}
Expand Down Expand Up @@ -151,7 +152,7 @@ var paymentBrokerExports = exec.Exports{
// CreateChannel creates a new payment channel from the caller to the target.
// The value attached to the invocation is used as the deposit, and the channel
// will expire and return all of its money to the owner after the given block height.
func (pb *Actor) CreateChannel(ctx *vm.Context, target types.Address, eol *types.BlockHeight) (*types.ChannelID, uint8, error) {
func (pb *Actor) CreateChannel(ctx *vm.Context, target address.Address, eol *types.BlockHeight) (*types.ChannelID, uint8, error) {
var state State
ret, err := actor.WithState(ctx, &state, func() (interface{}, error) {
// require that from account be an account actor to ensure nonce is a valid id
Expand Down Expand Up @@ -201,7 +202,7 @@ func (pb *Actor) CreateChannel(ctx *vm.Context, target types.Address, eol *types
// target Update(200) -> Payer: 1000, Target: 200, Channel: 800
// target Close(500) -> Payer: 1500, Target: 500, Channel: 0
//
func (pb *Actor) Update(ctx *vm.Context, payer types.Address, chid *types.ChannelID, amt *types.AttoFIL, sig Signature) (uint8, error) {
func (pb *Actor) Update(ctx *vm.Context, payer address.Address, chid *types.ChannelID, amt *types.AttoFIL, sig Signature) (uint8, error) {
var state State
_, err := actor.WithState(ctx, &state, func() (interface{}, error) {
data := createVoucherSignatureData(chid, amt)
Expand All @@ -226,7 +227,7 @@ func (pb *Actor) Update(ctx *vm.Context, payer types.Address, chid *types.Channe

// Close first executes the logic performed in the the Update method, then returns all
// funds remaining in the channel to the payer account and deletes the channel.
func (pb *Actor) Close(ctx *vm.Context, payer types.Address, chid *types.ChannelID, amt *types.AttoFIL, sig Signature) (uint8, error) {
func (pb *Actor) Close(ctx *vm.Context, payer address.Address, chid *types.ChannelID, amt *types.AttoFIL, sig Signature) (uint8, error) {
var state State
_, err := actor.WithState(ctx, &state, func() (interface{}, error) {

Expand Down Expand Up @@ -347,7 +348,7 @@ func (pb *Actor) Voucher(ctx *vm.Context, chid *types.ChannelID, amount *types.A

// Ls returns all payment channels for a given payer address.
// The slice of channels will be returned as cbor encoded map from string channelId to PaymentChannel.
func (pb *Actor) Ls(ctx *vm.Context, payer types.Address) ([]byte, uint8, error) {
func (pb *Actor) Ls(ctx *vm.Context, payer address.Address) ([]byte, uint8, error) {
var state State
ret, err := actor.WithState(ctx, &state, func() (interface{}, error) {
byPayer, found := state.Channels[payer.String()]
Expand All @@ -364,7 +365,7 @@ func (pb *Actor) Ls(ctx *vm.Context, payer types.Address) ([]byte, uint8, error)
return ret.([]byte), 0, nil
}

func findChannel(state *State, payer types.Address, chid *types.ChannelID) (*PaymentChannel, error) {
func findChannel(state *State, payer address.Address, chid *types.ChannelID) (*PaymentChannel, error) {
actorsChannels, found := state.Channels[payer.String()]
if !found {
return nil, Errors[ErrUnknownChannel]
Expand All @@ -378,7 +379,7 @@ func findChannel(state *State, payer types.Address, chid *types.ChannelID) (*Pay
return channel, nil
}

func updateChannel(ctx *vm.Context, target types.Address, channel *PaymentChannel, amt *types.AttoFIL) error {
func updateChannel(ctx *vm.Context, target address.Address, channel *PaymentChannel, amt *types.AttoFIL) error {
if target != channel.Target {
return Errors[ErrWrongTarget]
}
Expand Down Expand Up @@ -408,7 +409,7 @@ func updateChannel(ctx *vm.Context, target types.Address, channel *PaymentChanne
return nil
}

func reclaim(ctx *vm.Context, state *State, payer types.Address, chid *types.ChannelID, channel *PaymentChannel) error {
func reclaim(ctx *vm.Context, state *State, payer address.Address, chid *types.ChannelID, channel *PaymentChannel) error {
amt := channel.Amount.Sub(channel.AmountRedeemed)
if amt.LessEqual(types.ZeroAttoFIL) {
return nil
Expand Down Expand Up @@ -441,7 +442,7 @@ const separator = 0x0
// SignVoucher creates the signature for the given combination of
// channel, amount and from address.
// It does so by sign the following bytes: (channelID | 0x0 | amount)
func SignVoucher(channelID *types.ChannelID, amount *types.AttoFIL, addr types.Address, signer types.Signer) (types.Signature, error) {
func SignVoucher(channelID *types.ChannelID, amount *types.AttoFIL, addr address.Address, signer types.Signer) (types.Signature, error) {
data := createVoucherSignatureData(channelID, amount)
return signer.SignBytes(data, addr)
}
Expand Down
Loading

0 comments on commit efdc601

Please sign in to comment.