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

{WIP} add abstraction over builders for 1.5 and 2.0 #489

Merged
merged 9 commits into from
Jan 22, 2025

Conversation

eugenebelov
Copy link
Collaborator

No description provided.

@eugenebelov eugenebelov requested review from ihor and Comp0te January 17, 2025 14:38
.byHash(contractHash)
.entryPoint('redelegate')
.chainName(networkName)
// .amount(amountMotes)
Copy link

Choose a reason for hiding this comment

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

Accidental comment?

.buildFor1_5();
}

return new Error('Need to provide contract hash');
Copy link

Choose a reason for hiding this comment

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

Let's change to Auction contract hash is required when creating a transaction on Casper Network 1.5.x

amountMotes: string | BigNumber,
deployCost: number,
ttl: number,
contractHash?: string
Copy link

Choose a reason for hiding this comment

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

Let's rename to auctionContractHash

.buildFor1_5();
}

return new Error('Need to provide contract hash');
Copy link

Choose a reason for hiding this comment

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

Let's change to Auction contract hash is required when creating a transaction on Casper Network 1.5.x

amountMotes: string | BigNumber,
deployCost: number,
ttl: number,
contractHash?: string
Copy link

Choose a reason for hiding this comment

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

Let's rename to auctionContractHash

src/utils/cspr-network.ts Outdated Show resolved Hide resolved
src/utils/cspr-network.ts Outdated Show resolved Hide resolved
src/utils/cspr-network.ts Outdated Show resolved Hide resolved
return await this.rpcClient.getTransactionByDeployHash(
hash.deploy.toHex()
);
}
Copy link

Choose a reason for hiding this comment

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

I suppose this section should go inside the version 2 condition and the getDeploy RPC method to be used for 1.5.x

Copy link

@ihor ihor left a comment

Choose a reason for hiding this comment

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

@eugenebelov please check my comments

@eugenebelov eugenebelov requested a review from ihor January 20, 2025 15:47
Copy link

@ihor ihor left a comment

Choose a reason for hiding this comment

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

@eugenebelov could you please also add return types to all methods

@eugenebelov eugenebelov marked this pull request as ready for review January 20, 2025 16:10
@eugenebelov eugenebelov requested a review from ihor January 20, 2025 16:11
src/utils/cspr-network.ts Outdated Show resolved Hide resolved
Copy link

@ihor ihor left a comment

Choose a reason for hiding this comment

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

@eugenebelov generic createTransaction method is missing

Copy link

@davidatwhiletrue davidatwhiletrue left a comment

Choose a reason for hiding this comment

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

@eugenebelov see my comments

src/utils/cspr-network.ts Outdated Show resolved Hide resolved
src/utils/cspr-network.ts Outdated Show resolved Hide resolved
@eugenebelov eugenebelov requested a review from ihor January 21, 2025 15:20
src/utils/cspr-network.ts Outdated Show resolved Hide resolved
@eugenebelov eugenebelov requested a review from ihor January 22, 2025 11:11
@Comp0te Comp0te merged commit 3ea58a6 into feat-5.0.0 Jan 22, 2025
1 check passed
@eugenebelov eugenebelov deleted the feature/CSDK-206-add-abstraction-over-builders branch January 22, 2025 14:10
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.

5 participants