Optimise storage for simple core. #504
Milestone
Comments
schemar
added a commit
to schemar/mosaic-contracts
that referenced
this issue
Dec 13, 2018
The safe core now only tracks a fixed number of most recent state roots. The number of state roots to track can be set on construction. The safe core has a ring buffer that tracks the last `n` state roots and from `n + 1` starts overwriting the oldest state root whenever a new state root is committed. I created a separate contract for the circular buffer in order to decouple the concept of a circular buffer and the concept of a state root storage. There were three options of how to add the circular buffer logic to the state root: 1. Deploy the buffer as a separate contract (on core construction) and call to that contract for storage. 2. Use dependency injection and pass the address of the buffer when constructing the core. 3. Extend the buffer with the core to make the logic available. 4. Make the buffer a library. Option 4. was quickly ruled out, as the storage part of the buffer is essential to its logic and inner workings. Options 1. and 2. are somewhat similar. They very well decouple the contracts and make testing easy. From an architectural point of view, option 2. is the easy choice. However, in solidity it does cost actual money to split contracts and make external calls. So that's the drawback. Option 3. puts everything into a single contract and makes all calls to the buffer internal, which is good from an economical point of view. The drawback is of cours that the contracts are very tightly coupled and testing is a pain, as a wrapper contract must be written for the buffer to "externalize" its methods. After a discussion with @benjaminbollen and @pgev, I was convinced to choose the economically more viable approach over the better architectural approach. Core now extends the buffer and makes `internal` calls to it. I decided to leave the contract name in front of all the method names to make it clear to the reader of the contract when the buffer is used. I added tests for the circular buffer and also added a test specific to the state roots to the safe core. Fixes OpenST#504
schemar
added a commit
to schemar/mosaic-contracts
that referenced
this issue
Dec 13, 2018
The safe core now only tracks a fixed number of most recent state roots. The number of state roots to track can be set on construction. The safe core has a ring buffer that tracks the last `n` state roots and from `n + 1` starts overwriting the oldest state root whenever a new state root is committed. I created a separate contract for the circular buffer in order to decouple the concept of a circular buffer and the concept of a state root storage. There were three options of how to add the circular buffer logic to the state root: 1. Deploy the buffer as a separate contract (on core construction) and call to that contract for storage. 2. Use dependency injection and pass the address of the buffer when constructing the core. 3. Extend the buffer with the core to make the logic available. 4. Make the buffer a library. Option 4. was quickly ruled out, as the storage part of the buffer is essential to its logic and inner workings. Options 1. and 2. are somewhat similar. They very well decouple the contracts and make testing easy. From an architectural point of view, option 2. is the easy choice. However, in solidity it does cost actual money to split contracts and make external calls. So that's the drawback. Option 3. puts everything into a single contract and makes all calls to the buffer internal, which is good from an economical point of view. The drawback is of cours that the contracts are very tightly coupled and testing is a pain, as a wrapper contract must be written for the buffer to "externalize" its methods. After a discussion with @benjaminbollen and @pgev, I was convinced to choose the economically more viable approach over the better architectural approach. Core now extends the buffer and makes `internal` calls to it. I decided to leave the contract name in front of all the method names to make it clear to the reader of the contract when the buffer is used. I added tests for the circular buffer and also added a test specific to the state roots to the safe core. Fixes OpenST#504
schemar
added a commit
to schemar/mosaic-contracts
that referenced
this issue
Dec 13, 2018
The safe core now only tracks a fixed number of most recent state roots. The number of state roots to track can be set on construction. The safe core has a ring buffer that tracks the last `n` state roots and from `n + 1` starts overwriting the oldest state root whenever a new state root is committed. I created a separate contract for the circular buffer in order to decouple the concept of a circular buffer and the concept of a state root storage. There were three options of how to add the circular buffer logic to the state root: 1. Deploy the buffer as a separate contract (on core construction) and call to that contract for storage. 2. Use dependency injection and pass the address of the buffer when constructing the core. 3. Extend the buffer with the core to make the logic available. 4. Make the buffer a library. Option 4. was quickly ruled out, as the storage part of the buffer is essential to its logic and inner workings. Options 1. and 2. are somewhat similar. They very well decouple the contracts and make testing easy. From an architectural point of view, option 2. is the easy choice. However, in solidity it does cost actual money to split contracts and make external calls. So that's the drawback. Option 3. puts everything into a single contract and makes all calls to the buffer internal, which is good from an economical point of view. The drawback is of cours that the contracts are very tightly coupled and testing is a pain, as a wrapper contract must be written for the buffer to "externalize" its methods. After a discussion with @benjaminbollen and @pgev, I was convinced to choose the economically more viable approach over the better architectural approach. Core now extends the buffer and makes `internal` calls to it. I decided to leave the contract name in front of all the method names to make it clear to the reader of the contract when the buffer is used. I added tests for the circular buffer and also added a test specific to the state roots to the safe core. Fixes OpenST#504
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Simple core shouldn't track all the state root.
It should track a fixed number of latest state roots. The number should be set on construction.
The text was updated successfully, but these errors were encountered: