-
Notifications
You must be signed in to change notification settings - Fork 467
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
Use leb128 encoding for token amounts #571
Conversation
ta := NewZeroAttoFIL() | ||
_, ok := ta.val.SetString(s, base) | ||
return ta, ok | ||
af := NewZeroAttoFIL() |
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.
that's gonna be a fun abbreviation
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.
LGTM
actor/builtin/miner/miner_test.go
Outdated
@@ -52,7 +52,7 @@ func TestAddAsk(t *testing.T) { | |||
|
|||
result, err := core.ApplyMessage(ctx, st, msg, types.NewBlockHeight(0)) | |||
assert.NoError(err) | |||
assert.Equal(types.NewAttoFILFromFIL(0), types.NewAttoFILFromBytes(result.Receipt.Return[0])) | |||
assert.True(big.NewInt(0).Cmp(big.NewInt(0).SetBytes(result.Receipt.Return[0])) == 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.
nit: can we keep this as an Equal
check? I like seeing expect: 0 actual: -1
in test failures better than telling me something was false
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.
done
types/atto_fil.go
Outdated
@@ -6,6 +6,7 @@ import ( | |||
"math/big" | |||
|
|||
cbor "gx/ipfs/QmRiRJhn427YVuufBEHofLreKWNw7P7BWNq86Sb9kzqdbd/go-ipld-cbor" | |||
leb128 "gx/ipfs/QmSKyB5faguXT4NqbrXpnRXqaVj5DhSm7x9BtzFydBY1UK/go-leb128" |
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.
nit: adding leb128
is redundant (cbor
above is as well)
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.
learn something new every day. my editor wont let me drop the cbor one but it lets me drop the leb128, blame goreturns.
This work towards #340.