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

Some clarification after implementation #533

Merged
merged 5 commits into from
Nov 30, 2023
Merged

Conversation

xgreenx
Copy link
Contributor

@xgreenx xgreenx commented Nov 7, 2023

Findings after implementation FuelLabs/fuel-vm#623

@xgreenx xgreenx requested review from Voxelot and a team November 7, 2023 10:50
@xgreenx xgreenx self-assigned this Nov 7, 2023
Dentosal
Dentosal previously approved these changes Nov 8, 2023
MitchTurner
MitchTurner previously approved these changes Nov 8, 2023
@xgreenx xgreenx dismissed stale reviews from MitchTurner and Dentosal via b4bf86f November 8, 2023 09:23
| `outputs` | [Output](./output.md)`[]` | List of outputs. |
| `witnesses` | [Witness](./witness.md)`[]` | List of witnesses. |

Given helper `max_gas()` returns the maximum gas that the transaction can use.
Copy link
Member

@Voxelot Voxelot Nov 8, 2023

Choose a reason for hiding this comment

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

Do we define max_gas() anywhere? It might be confusing without more context. i.e in tx-validity

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 it is what you've started in the #529 =D I decided not to describe it here because we have another PR dedicated to the gas model description.

@xgreenx xgreenx enabled auto-merge (squash) November 29, 2023 17:04
@xgreenx xgreenx requested review from Voxelot, Dentosal, MitchTurner and a team November 30, 2023 11:09
@xgreenx xgreenx merged commit 0904457 into master Nov 30, 2023
5 checks passed
@xgreenx xgreenx deleted the feature/some-clarification branch November 30, 2023 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants