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.

Sign up for GitHub

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

Do not automatically create cip66 transactions ever #27

Merged
merged 2 commits into from
Aug 13, 2024
Merged

Do not automatically create cip66 transactions ever #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
76a97a6
Whereas originally also cel2 upgraded chains were supposed to have su…
aaronmgdr Aug 13, 2024
d0f471d
updated tests to reflect new reality that 66 is never automatically c…
aaronmgdr Aug 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/lib/CeloWallet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,8 @@ export default class CeloWallet extends Wallet {
// sets feedata for the transaction.
//
async populateFees(tx: CeloTransactionRequest) {
const isCel2 = await this.isCel2();
const noFeeCurrency = !tx.feeCurrency;
const useCIP66ForEasyFeeTransactions = isCel2 && !noFeeCurrency;
const useCIP66ForEasyFeeTransactions = false // cip66 is not yet implemented in nodes
// CIP 66 transactions are denominated in CELO not the fee token
const feesAreInCELO = noFeeCurrency || useCIP66ForEasyFeeTransactions;

Expand Down Expand Up @@ -175,12 +174,13 @@ export default class CeloWallet extends Wallet {
}

/**
* For cip 66 transactions (the prefered way to pay for gas with fee tokens on Cel2) it is necessary
* to provide the absolute limit one is willing to pay denominated in the token.
* @remarks For cip 66 transactions it is necessary to provide the absolute limit one is willing to pay denominated in the token.
* In contrast with earlier tx types for fee currencies (celo legacy, cip42, cip 64).
*

* Calulating Estimation requires the gas, maxfeePerGas and the conversion rate from CELO to feeToken
* https://github.com/celo-org/celo-proposals/blob/master/CIPs/cip-0066.md
*
* @dev cip66 has yet to be implemented on celo nodes. Dont add maxFeeInFeeCurrency to your transaction. use cip64
*/
async estimateMaxFeeInFeeToken({
gasLimit,
Expand Down
4 changes: 2 additions & 2 deletions tests/CeloWallet.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe("CeloWallet", () => {
});
});
describe("when feeCurrency on cel2", () => {
test("populates maxFeePerGas and maxPriorityFeePerGas and maxFeeInFeeCurrency", async () => {
test("populates maxFeePerGas and maxPriorityFeePerGas (not maxFeeInFeeCurrency)", async () => {
const wallet = getSigner();

jest.spyOn(wallet, "isCel2").mockImplementation(async () => true);
Expand All @@ -36,7 +36,7 @@ describe("CeloWallet", () => {
expect(typeof filled.maxFeePerGas).toEqual("bigint");
expect(typeof filled.maxPriorityFeePerGas).toEqual("bigint");

expect(typeof filled.maxFeeInFeeCurrency).toEqual("bigint");
expect(filled.maxFeeInFeeCurrency).toBeUndefined();
expect(filled.feeCurrency).toEqual(USDC_ADAPTER_ALFAJORES_ADDRESS);
});
});
Expand Down
Loading