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

Support for non-18 decimals in custom fee token #24

Merged
merged 18 commits into from
Jul 9, 2024

Conversation

gvladika
Copy link
Contributor

@gvladika gvladika commented Jan 8, 2024

Add --l3-fee-token-decimals flag to support custom number of decimals in Orbit's fee token.

Ie. this will spin up an Orbit chain with 6 decimals fee token:

./test-node.bash --init --l3node --l3-fee-token --l3-fee-token-decimals 6

@gvladika gvladika changed the base branch from use-tokenbridge-creator to master February 1, 2024 17:39
@gvladika gvladika requested a review from gzeoneth February 8, 2024 13:47
@gvladika gvladika marked this pull request as ready for review February 9, 2024 17:45
@gvladika gvladika requested a review from tsahee March 8, 2024 13:01
@gvladika gvladika changed the base branch from master to decouple-contracts April 19, 2024 13:15
@gvladika gvladika marked this pull request as draft April 23, 2024 11:12
@gvladika gvladika marked this pull request as ready for review April 23, 2024 11:47
@gvladika gvladika changed the base branch from decouple-contracts to release April 29, 2024 14:00
@gvladika gvladika requested a review from godzillaba May 9, 2024 13:16
@gzeoneth
Copy link
Member

gzeoneth commented Jul 4, 2024

pushed a nodejs v18 commit to non18-decimal-token-node-18 branch on top of this

tsahee
tsahee previously approved these changes Jul 8, 2024
Copy link
Collaborator

@tsahee tsahee left a comment

Choose a reason for hiding this comment

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

seems good. One small suggestion but not a must.

@@ -77,14 +82,42 @@ async function bridgeNativeToken(argv: any, parentChainUrl: string, chainUrl: st
const sleep = (ms: number) => new Promise(r => setTimeout(r, ms));
while (true) {
const balance = await bridger.getBalance()
if (balance.gte(ethers.utils.parseEther(argv.amount))) {
if (balance.gte(0)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

.gte(depositAmount)?

gte(0) will not work for 2nd transfer, gte(depositAmount) will at least work if 2nd transfer is larger than previous balance which seems reasonable.

there are probably better ways

Copy link
Contributor Author

@gvladika gvladika Jul 9, 2024

Choose a reason for hiding this comment

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

In case when fee token has non-18 decimals, depositAmount is not equal to the amount minted on L2 (we do inflation/deflation to match 18-decimals denomination for native currency). I've improved this check to properly wait for expected amount to be minted on L2 3af86a0

@gvladika gvladika merged commit e4ff0cf into release Jul 9, 2024
1 check passed
Sneh1999 pushed a commit to Sneh1999/nitro-testnode that referenced this pull request Jul 19, 2024
Set simple is false when running espresso
@tsahee tsahee deleted the non18-decimal-token branch August 8, 2024 22:12
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.

3 participants