-
Notifications
You must be signed in to change notification settings - Fork 5
Export fields #13
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
base: master
Are you sure you want to change the base?
Export fields #13
Conversation
- Exported fields of MyStruct to enable encoding and decoding with the encoding/gob package.
| type Int struct { | ||
| abs *uint256.Int | ||
| neg bool | ||
| Abs *uint256.Int |
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.
Why did we put it public?
We don't want to leak and access these components from outside or other places that will use this package.
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.
Allowing the Int struct fields to be exported enables serialization with packages such as Gob, JSON, XML, etc. If the fields aren’t exported, serialization will result in an empty struct because these packages cannot access unexported fields.
I believe that having exported fields isn't an issue since their behavior is straightforward. In the uint256 library, the four uint64 fields that make up the type are also exported and can be modified outside of the package.
// Int is represented as an array of 4 uint64, in little-endian order,
// so that Int[3] is the most significant, and Int[0] is the least significant
type Int [4]uint64
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.
Hi @louisgthier
Can you give me an example for this case?
Allowing the Int struct fields to be exported enables serialization with packages such as Gob, JSON, XML, etc
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.
Allowing the Int struct fields to be exported enables serialization with packages such as Gob, JSON, XML, etc. If the fields aren’t exported, serialization will result in an empty struct because these packages cannot access unexported fields.
I believe that having exported fields isn't an issue since their behavior is straightforward. In the uint256 library, the four uint64 fields that make up the type are also exported and can be modified outside of the package.
// Int is represented as an array of 4 uint64, in little-endian order, // so that Int[3] is the most significant, and Int[0] is the least significant type Int [4]uint64
Regarding the Json serialization:
- To allow a struct to be serialized to JSON or unserialized from JSON in Go, you generally need to create methods that implement the Marshaler and Unmarshaler interfaces defined in the encoding/json package. So no need to expose those fields, the implemention of those interfaces is enough.
Almost sure something simillar exit for other XML, Gob etc format but I'm not familiar with those.
- "big.Int" have a struct similar to this package and those fields aren't exported: https://go.dev/src/math/big/int.go
- To marshal "big.int" to Json, they implement the interfaces needed for that here: https://go.dev/src/math/big/intmarsh.go
|
Here is an example of JSON serialization with and without exporting fields of int256.Int for some UniswapV3Pool struct: When exporting fields: When keeping them private: The only *int256.Int here is LiquidityNet in the Ticks, and as you can see they are lost when fields are not exported. Same thing happens with Gob: With exported fields:
Without exported fields:
|
|
I've already worked on UniswapV2 data and faced the same problem:
I'm wsorking on a pull request that will add those two interfaces, in addition to MarshalText() and UnmarshalText(input []byte) interface. Edit: I just push the pull request, not sure if you can pull it and see if it solve the json encoding/decoding problem for you. |
|
@louisgthier As I understand, we need to implement JSON marshaller/unmarshaller functions. |
I've added those functions in my last pull request #14. And included an example of Json marshalling/unmarshalling working correctly as intended without exposing those fields. ```go
type TickInfo struct {
Initialized bool
LiquidityNet *int256.Int
}
type Pool struct {
PoolAddress *common.Address
Ticks map[int]*TickInfo
}
func main() {
tick1Info := &TickInfo{
Initialized: true,
LiquidityNet: int256.MustFromDecimal("-111000000000000000000000000000000000099990"),
}
tick2Info := &TickInfo{
Initialized: true,
LiquidityNet: int256.MustFromDecimal("1110000000000440000000000000000000000099990"),
}
ticks := make(map[int]*TickInfo)
ticks[100] = tick2Info
ticks[-100] = tick1Info
poolAddr := common.HexToAddress("0x0000000000000000000000000000000000000000")
pool := &Pool{
PoolAddress: &poolAddr,
Ticks: ticks,
}
data, err := json.Marshal(pool)
if err != nil {
fmt.Println(err)
} else {
// {"PoolAddress":"0x0000000000000000000000000000000000000000","Ticks":{"-100":{"Initialized":true,"LiquidityNet":"-111000000000000000000000000000000000099990"},"100":{"Initialized":true,"LiquidityNet":"1110000000000440000000000000000000000099990"}}}
fmt.Println(string(data))
}
newPool := &Pool{}
poolData := "{\"PoolAddress\":\"0x0000000000000000000000000000000000000000\",\"Ticks\":{\"-100\":{\"Initialized\":true,\"LiquidityNet\":\"-111000000000000000000000000000000000099990\"},\"100\":{\"Initialized\":true,\"LiquidityNet\":\"1110000000000440000000000000000000000099990\"}}}"
err = json.Unmarshal([]byte(poolData), &newPool)
if err != nil {
fmt.Println(err)
} else {
// -111000000000000000000000000000000000099990
fmt.Println(newPool.Ticks[-100].LiquidityNet.String())
// 1110000000000440000000000000000000000099990
fmt.Println(newPool.Ticks[100].LiquidityNet.String())
}
}Not Familiar with Gob tho to try and implement the methods needed for encoding/decoding that format. |
|
@louisgthier can u tell me the progress for this PR? |
I have been using exported fields in my fork for simplicity but I just took a look at how decimal.Decimal handles Gob encoding/decoding and I'll try to do the same. It will probably be in another PR though. |
Exported fields of Int to enable encoding and decoding with the encoding/gob, encoding/json... packages.