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

feat: p-chain dynamic fees #872

Merged
merged 40 commits into from
Sep 27, 2024
Merged

Conversation

erictaylor
Copy link
Member

@erictaylor erictaylor commented Aug 14, 2024

This is a first draft of changes needed for upcoming p-chain dynamic fees.

This PR introduces new utils and a new builder that is currently exposed alongside the existing builder. All new builder functions are currently exported under a etna specific directory.

The new builder does not include any transactions types that are not supported post Etna upgrade.

Tests are added that align for the most part to the tests in avalanchego. However, the builder tests are a bit of a hybrid between existing builder tests and new changes needed for dynamic fee testing.

Things this PR does not do (will come later)

  • Pulling gas price and weights from API to set in context.
  • Calculating "compute" complexity of transactions.

Things not planned (currently)

  • This does not touch the X-chain. We are following in avalanchego's steps here, and the X-chain hasn't been worked on. Though the idea is that the changes will look very similar and there could be a lot of code reuse.

Extra changes that do not effect current implementation.

  • Changed a bunch of types (for arrays) to be readonly. Mostly to keep my sanity that certain values were immutable. I wanted that extra piece of mind. All of the places readonly is added it doesn't effect existing implementations and was already being treated as such.
  • Fixing some spelling mistakes as I was going along reading existing code.
  • Adds a cspell config but does not enforce it. Also added the CSpell extension recommendation to VSCode. Helps with spelling mistakes. ;)

@erictaylor erictaylor added DO NOT MERGE Still a work in progress wip labels Aug 14, 2024
@erictaylor erictaylor self-assigned this Aug 14, 2024
src/vms/context/context.ts Outdated Show resolved Hide resolved
@erictaylor erictaylor removed the DO NOT MERGE Still a work in progress label Sep 5, 2024
Copy link
Collaborator

@rictorlome rictorlome left a comment

Choose a reason for hiding this comment

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

Thank you, this is really coming along! I left some more notes

src/crypto/secp256k1.ts Show resolved Hide resolved
src/vms/pvm/etna-builder/spend.ts Outdated Show resolved Hide resolved
src/vms/pvm/etna-builder/spendHelper.ts Show resolved Hide resolved
src/vms/pvm/etna-builder/spendHelper.ts Outdated Show resolved Hide resolved
src/vms/pvm/etna-builder/spendHelper.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@bferenc bferenc left a comment

Choose a reason for hiding this comment

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

Awesome work! Haven't checked everything yet, but wanted to stop and share a few questions regarding the useUnlockedUTXOs. Please take a look at my comments below when you have a moment 👇

Copy link
Collaborator

@bferenc bferenc left a comment

Choose a reason for hiding this comment

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

Awesome work! 👏

@rictorlome rictorlome self-requested a review September 27, 2024 17:34
Copy link
Collaborator

@rictorlome rictorlome left a comment

Choose a reason for hiding this comment

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

Great job!

src/vms/pvm/etna-builder/builder.test.ts Outdated Show resolved Hide resolved
@erictaylor erictaylor merged commit 5ae4440 into master Sep 27, 2024
3 checks passed
@erictaylor erictaylor deleted the erictaylor/p-chain-dynamic-fees branch September 27, 2024 17:38
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