Improve readability #84
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kayibal
reviewed
Oct 24, 2024
|
|
||
| * **swap(uint256 givenAmount, bytes calldata data)** | ||
| * **Purpose**: To perform a token swap, either specifying the input amount to get the output amount or vice versa. | ||
| * **Purpose**: Perform a token swap. |
There was a problem hiding this comment.
It omitted quite some information here...
| * **Returns**: The amount of the token swapped. | ||
| * `givenAmount`: Token amount (input or output) for the swap. | ||
| * `data`: Encoded swap information from `SwapStructEncoder`. | ||
| * **Returns**: Swapped token amount. |
There was a problem hiding this comment.
We usually call this the calculated (owned or received) amount.
| 5. **Token Transfer Support**: Ensure that the implementation supports transferring received tokens to a designated receiver address, either within the swap function or through an additional transfer step. | ||
| 6. **Gas Efficiency**: Ensure the implementation is gas-efficient. Strive for optimal performance in the swap logic. The usage of assembly is not necessary. | ||
| 7. **Security Considerations**: Follow common security best practices, such as validating inputs, ensuring proper access control, and safeguarding against reentrancy attacks. | ||
| 1. **Define Protocol Logic**: Implement `swap` function to interact with the protocol's liquidity pool. Use `data` to encode pool and token addresses. |
There was a problem hiding this comment.
This sentence changed quite significantly, it should convey that data can contain anything including pool and token addresses (if those are necessary to perform the swap)
|
|
||
| Before continuing though, it is important to understand the concept of map modules and store modules. You can read about those [here](https://substreams.streamingfast.io/documentation/develop/manifest-modules/types). |
There was a problem hiding this comment.
This paragraph was completely omitted, I wonder why? I think this piece of information is quite helpful.
|
|
||
| In the following section, we outline the typical procedure for structuring an integration for the Virtual Machine (VM) implementation type: | ||
|
|
||
| Commonly, protocols employ factory contracts to deploy a contract for each swap component (also known as a pool or pair). We will explain how to efficiently index this system of contracts. It's important to note, however, that the techniques described below are broadly applicable, even if a protocol deviates from this specific pattern it should be possible to index it and emit the required messages. |
There was a problem hiding this comment.
Also I feel the information from this is mostly lost... e.g. it reminds the user again what is considered a component...
| Usually an integration consists of the following modules: | ||
|
|
||
| * `map_components(block)` | ||
| * This map module extracts any newly created components by inspecting the block model (e.g factory contract logs). The recommended output model for this module is `BlockTransactionProtocolComponents.` |
There was a problem hiding this comment.
Recommendation is now missing.
| * `map_components(block)` | ||
| * This map module extracts any newly created components by inspecting the block model (e.g factory contract logs). The recommended output model for this module is `BlockTransactionProtocolComponents.` | ||
| * `store_components(components, components_store)` | ||
| * This store module takes the detected components and stores any necessary information about the component for downstream modules. For vm integrations the address is most likely enough. |
There was a problem hiding this comment.
Also here things are missing, I think it is too short now... it's hard for people to understand how this works without being a bit more concrete and giving examples imo.
Comment on lines
+40
to
+52
| 1. Implement a `factory.rs` module to detect newly deployed components. | ||
| 2. Use `BlockTransactionProtocolComponents` as the output model: | ||
| ```protobuf | ||
| message TransactionProtocolComponents { | ||
| Transaction tx = 1; | ||
| repeated ProtocolComponent components = 2; | ||
| } | ||
|
|
||
| Later we'll have to emit balance and state changes based on the set of currently tracked components. | ||
| message BlockTransactionProtocolComponents { | ||
| repeated TransactionProtocolComponents tx_components = 1; | ||
| } | ||
| ``` | ||
| 3. Store component addresses for downstream use. |
There was a problem hiding this comment.
Idk, it completely oversimplified things here...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Improve simplicity, brevity, clarity, organization, and change passive to active voice