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

Added Ethereum-compatible legacy TXs support #182

Merged
merged 3 commits into from
Mar 21, 2024

Conversation

carterqw2
Copy link
Contributor

Description

Celo legacy Type 0 transactions were deprecated in the Gingerbread hardfork. As a first step, transactions that don't require alternative fee currencies should fallback to Ethereum-compatible TXs that are still supported.

Other changes

Fixed a typo in the check.

Tested

Added tests.

Backwards compatibility

Yes.

Copy link

changeset-bot bot commented Mar 12, 2024

🦋 Changeset detected

Latest commit: 3cf7e47

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
@celo/connect Minor
@celo/wallet-base Minor
@celo/celocli Patch
@celo/contractkit Patch
@celo/explorer Patch
@celo/governance Patch
@celo/transactions-uri Patch
@celo/wallet-hsm-aws Minor
@celo/wallet-hsm-azure Minor
@celo/wallet-hsm-gcp Minor
@celo/wallet-ledger Minor
@celo/wallet-local Minor
@celo/wallet-remote Minor
@celo/wallet-rpc Minor
@celo/keystores Patch
@celo/wallet-hsm Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@aaronmgdr
Copy link
Member

looks good as far as i see.

have you sent a transaction over the network with this?

@carterqw2
Copy link
Contributor Author

carterqw2 commented Mar 18, 2024

I updated a local instance of celo-oracle to use this version and sent a test transaction:

    {
      blockHash: '0x571653091dbe9769112443e38552ac9ded429dc2d2d773121cca8f23eef1d50e',
      blockNumber: 99562,
      from: '0xF3e322E4Bf983916B25582467f69F96619472a07',
      gas: 252798,
      gasPrice: '150000000',
      feeCurrency: null,
      gatewayFeeRecipient: null,
      gatewayFee: '0x0',
      hash: '0x58931086c84c58f17a7ff02e65afcbe9732ed61c9930e70a7187f6fef18aacf6',
      input: '0x80e50744000000000000000000000000000000000000000000000000000000000000d008000000000000000000000000000000000000000000084595161401484a00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000',
      nonce: 91,
      to: '0x000000000000000000000000000000000000D004',
      transactionIndex: 0,
      value: '0',
      type: 0,
      v: '0x1583f3',
      r: '0xc13cdc66509f1cab21b41eeb0d607ae857cc84f50636a66fd5f2998e63652dec',
      s: '0x1add4c4a42b644d9a03dc426821d8fdf0ce34f61f92ec692656c6ce06218f08f',
      ethCompatible: true
    }

so seems to be working as expected.

What do I need to fix for the docs tests, do you know? The error is a bit vague.

@shazarre
Copy link
Contributor

@carterqw2 just run yarn docs and commit docs that have changed

@aaronmgdr
Copy link
Member

the pr is set to draft but seems like it actually ready.

@carterqw2 anything else you want to dod here?

@carterqw2
Copy link
Contributor Author

carterqw2 commented Mar 18, 2024

the pr is set to draft but seems like it actually ready.

@carterqw2 anything else you want to dod here?

@aaronmgdr I'll change it to Ready for review then. I noticed that I had to update a bunch of package.json file in this repo to make it work locally with celo-oracle, I was wondering how versioning is handled? Would it have to be a separate PR that updates the package version?

@carterqw2 carterqw2 marked this pull request as ready for review March 18, 2024 18:00
@carterqw2 carterqw2 requested a review from a team as a code owner March 18, 2024 18:00
@aaronmgdr
Copy link
Member

aaronmgdr commented Mar 19, 2024

the pr is set to draft but seems like it actually ready.
@carterqw2 anything else you want to dod here?

@aaronmgdr I'll change it to Ready for review then. I noticed that I had to update a bunch of package.json file in this repo to make it work locally with celo-oracle, I was wondering how versioning is handled? Would it have to be a separate PR that updates the package version?

I dont see any package.json changes – guess you havent commited them yet?

package versioning is done thru changesets. ive gone ahead and added one to this PR already. we dont modify the versions directly. (we do of course update external dependency versions as needed) Maybe you can push up on a separate branch the changes you made for it to work with oracles?

@carterqw2
Copy link
Contributor Author

carterqw2 commented Mar 20, 2024

the pr is set to draft but seems like it actually ready.
@carterqw2 anything else you want to dod here?

@aaronmgdr I'll change it to Ready for review then. I noticed that I had to update a bunch of package.json file in this repo to make it work locally with celo-oracle, I was wondering how versioning is handled? Would it have to be a separate PR that updates the package version?

I dont see any package.json changes – guess you havent commited them yet?

package versioning is done thru changesets. ive gone ahead and added one to this PR already. we dont modify the versions directly. (we do of course update external dependency versions as needed) Maybe you can push up on a separate branch the changes you made for it to work with oracles?

Thanks for adding the changeset @aaronmgdr! I've pushed my changes to this branch, so for celo-oracle to work I used @palango's debug branch with references to a local copy of the developer-tooling repo and updated the three references in this repo. So, I'm not sure what would be the right sequence of steps to update all the references correctly to make the Oracle's PR work.

@aaronmgdr
Copy link
Member

the only changes on that other branch i see are ones linking the packages to local files. which we would not want |

i think we merge this in. we are doing a beta of new very soon (days) and you can test that out

@aaronmgdr aaronmgdr changed the base branch from master to prerelease/mcrn March 20, 2024 15:02
@shazarre
Copy link
Contributor

@carterqw2 approved the PR ✅ please just resolve the conflicts and we can merge it

@carterqw2 carterqw2 force-pushed the carterqw2/celo-legacy-tx-deprecation branch from e6c67a5 to f940aec Compare March 20, 2024 17:00
@carterqw2 carterqw2 force-pushed the carterqw2/celo-legacy-tx-deprecation branch from f940aec to 3cf7e47 Compare March 20, 2024 17:52
@carterqw2
Copy link
Contributor Author

@aaronmgdr, @shazarre I rebased on top of the release branch.

@aaronmgdr aaronmgdr merged commit 5335af5 into prerelease/mcrn Mar 21, 2024
13 checks passed
@aaronmgdr aaronmgdr deleted the carterqw2/celo-legacy-tx-deprecation branch March 21, 2024 08:07
This was referenced Mar 21, 2024
@nicolasbrugneaux nicolasbrugneaux mentioned this pull request Mar 25, 2024
aaronmgdr pushed a commit that referenced this pull request Mar 26, 2024
- Remove stable token Inflation related methods and configs #186
- USDC related changes #151
- Added Ethereum-compatible legacy TXs support #182
- Update dependency cross-fetch to v3.1.5 [SECURITY] #168
- feat: ignores gasCurrency when passed to the config #198
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