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

String conversion from decimal (#108) #122

Merged
merged 51 commits into from
Feb 8, 2023
Merged
Changes from 1 commit
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
bb5f382
relaxed marshaling, slow base 10 parsing
Sep 25, 2022
fb12295
update version
Sep 25, 2022
6866965
add control to benchmark
Sep 25, 2022
55f7ffc
make the ci thing less angry
Sep 25, 2022
5689315
make the ci even less angy
Sep 25, 2022
3c7131f
add fuzz tests
elee1766 Nov 28, 2022
5a106b0
change map
elee1766 Nov 28, 2022
b296df7
magic
elee1766 Nov 28, 2022
cb2c910
clear z on SetFrombig, even if b is nil
elee1766 Nov 28, 2022
df6b926
Update conversion.go
elee1766 Dec 6, 2022
bd117d9
Update conversion.go
Tjudice Dec 14, 2022
048efc0
Merge pull request #1 from Tjudice/patch-1
elee1766 Dec 14, 2022
9a76ab7
base10: minor api change, added docs
holiman Dec 15, 2022
42313ad
ci, go.mod: require go 1.18+
holiman Dec 15, 2022
7e0467a
squashme: circle fixes
holiman Dec 15, 2022
7f61ee9
properly cgheck for overflow in fuzz
elee1766 Dec 15, 2022
1d8f726
number overflow test
elee1766 Dec 15, 2022
f4e6c19
added comments to explain fromBase10Long
elee1766 Dec 15, 2022
84510f7
Update base10.go
elee1766 Dec 15, 2022
6e4f914
Update base10.go
elee1766 Dec 15, 2022
e25ce45
lift up
elee1766 Dec 15, 2022
8420f46
change cutLength to local constant
elee1766 Dec 15, 2022
3fc5f08
better doc
elee1766 Dec 15, 2022
d6f32f6
base10: modify algorithm + fixup tests a bit
holiman Dec 15, 2022
8c858a1
base10: fuzzing found some bugs, which were fixed
holiman Dec 15, 2022
078a8e6
fuzzing: some improvements to the fuzzer
holiman Dec 15, 2022
2413444
add oss-fuzz fuzzer, rename base10 to decimal
holiman Dec 15, 2022
2e6c49f
decimal: fix conversion on base 0, fallback to big.Int
holiman Dec 16, 2022
765b02b
decimal: move some unused constants
holiman Dec 16, 2022
3430ee5
more failing tests
holiman Dec 16, 2022
cbdcdcb
decimal: remove SetString method
holiman Dec 16, 2022
38d53f3
decimal: simplify tests
holiman Dec 16, 2022
ca9a5ea
rm unused constants, fix benchmarks
holiman Dec 16, 2022
9518831
Update decimal.go
elee1766 Dec 17, 2022
ad5ba70
Update conversion.go
elee1766 Dec 17, 2022
301af32
change sql valuer
elee1766 Dec 20, 2022
b5a4906
remove extra struct def
elee1766 Dec 29, 2022
fb7ceef
testing: try to get 100% coverage again
holiman Dec 29, 2022
4ef03d7
lintfix
holiman Dec 29, 2022
5587dd6
fuzzing: fix up fuzzer for string conversion
holiman Dec 29, 2022
7ff0d8f
circle: try to get circleci fuzzing going
holiman Dec 29, 2022
5019647
conversion: more coverage + fix in Scan
holiman Dec 29, 2022
2f0b0b7
decimal_test: more coverage
holiman Dec 29, 2022
dcd75fd
go.mod: fuzzing dep
holiman Dec 29, 2022
0472e9b
conversion: consistent use of ptr receiver
holiman Dec 29, 2022
d55274d
circle: make use of restored corpus
holiman Dec 29, 2022
82fd325
properly parse scientific notation
elee1766 Jan 12, 2023
35d62f2
add some overflow cases for scan, and error on overflow
elee1766 Jan 27, 2023
88226a1
conversion minor nitpicks
holiman Feb 8, 2023
5ff06b0
conversion: fix test
holiman Feb 8, 2023
091c3f9
conversion_test: bring coverage back to 100
holiman Feb 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"io"
"math/big"
"math/bits"
"strings"
)

const (
Expand Down Expand Up @@ -565,7 +566,25 @@ func (dst *Int) Scan(src interface{}) error {
}
switch src := src.(type) {
case string:
return dst.SetFromDecimal(src)
splt := strings.SplitN(src, "e", 2)
if len(splt) < 2 {
return dst.SetFromDecimal(src)
}
err := dst.SetFromDecimal(splt[0])
if err != nil {
return err
}
if splt[1] == "0" {
return nil
}
Comment on lines +576 to +578
Copy link
Owner

Choose a reason for hiding this comment

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

1e0 should be 1, not 0 ... ?
Not sure what 0e0 is defined as, need to check that.

Copy link
Contributor Author

@elee1766 elee1766 Jan 20, 2023

Choose a reason for hiding this comment

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

isn't that what this will return? assume X = splt[0], Y = splt[1]

I think XeY woudl be X * 10 ^ Y so 0e0 should be 0 * 10^0 = 0

In the previous lines, i do dst.SetFromDecimal(X)

then if Y == 0, then i should just return, since X*10^0=X

Copy link
Owner

Choose a reason for hiding this comment

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

My bad, yes you're right

exp := new(Int)
err = exp.SetFromDecimal(splt[1])
if err != nil {
return err
}
exp.Exp(NewInt(10), exp)
dst.Mul(dst, exp)
Copy link
Owner

Choose a reason for hiding this comment

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

For Exp we have no overflow checks,

// Exp sets z = base**exponent mod 2**256, and returns z.

For Mul, we do have MulOverflow, so we could check that and return error if it does not fit. But it feels a bit wonky to just silently fail the conversion (because of the Exp) without signalling an error.

IMO, it would have been ok if the failure-mode had been:

  • The result is X mod 2^256, where X is the correct result.

But I think the possible failures are different .. (?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think you are right here, they will be different.

i think the popular postgresql scanner will error anyways if the value is too large to be scanned in, so I think that it is fine to do the same.

return nil
case []byte:
return dst.SetFromDecimal(string(src))
}
Expand Down