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: update token for upgradable transactions where implementation i… #348

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions packages/data-fetcher/src/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { TransferService } from "./transfer/transfer.service";
import { TokenService } from "./token/token.service";
import { JsonRpcProviderModule } from "./rpcProvider/jsonRpcProvider.module";
import { MetricsModule } from "./metrics";
import { UpgradableService } from "./upgradable/upgradable.service";

@Module({
imports: [
Expand All @@ -26,6 +27,7 @@ import { MetricsModule } from "./metrics";
providers: [
BlockchainService,
AddressService,
UpgradableService,
BalanceService,
TransferService,
TokenService,
Expand Down
39 changes: 39 additions & 0 deletions packages/data-fetcher/src/log/log.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,23 @@ import { TokenService, Token } from "../token/token.service";
import { AddressService } from "../address/address.service";
import { BalanceService } from "../balance/balance.service";
import { ContractAddress } from "../address/interface/contractAddress.interface";
import { UpgradableService } from "../upgradable/upgradable.service";
import { ProxyAddress } from "src/upgradable/interface/proxyAddress.interface";

describe("LogService", () => {
let logService: LogService;
let addressServiceMock: AddressService;
let balanceServiceMock: BalanceService;
let transferServiceMock: TransferService;
let tokenServiceMock: TokenService;
let upgradableServiceMock: UpgradableService;

beforeEach(async () => {
addressServiceMock = mock<AddressService>();
balanceServiceMock = mock<BalanceService>();
transferServiceMock = mock<TransferService>();
tokenServiceMock = mock<TokenService>();
upgradableServiceMock = mock<UpgradableService>();

const app = await Test.createTestingModule({
providers: [
Expand All @@ -42,6 +46,10 @@ describe("LogService", () => {
provide: TokenService,
useValue: tokenServiceMock,
},
{
provide: UpgradableService,
useValue: upgradableServiceMock,
},
],
}).compile();

Expand All @@ -61,6 +69,17 @@ describe("LogService", () => {
mock<ContractAddress>({ address: "0xD144ca8Aa2E7DFECD56a3CCcBa1cd873c8e5db58" }),
];

const upgradableAddresses = [
mock<ProxyAddress>({
address: "0xEBf9D3ead9A8c2bb8cEa438B8Dfa9f1AFf44bfa7",
implementationAddress: "0xf43624d811c5DC9eF91cF237ab9B8eE220D438eE",
}),
mock<ProxyAddress>({
address: "0xdc187378edD8Ed1585fb47549Cc5fe633295d571",
implementationAddress: "0xD144ca8Aa2E7DFECD56a3CCcBa1cd873c8e5db58",
}),
];

const transfers = [
{ from: "from1", to: "to1", logIndex: 0 } as Transfer,
{ from: "from2", to: "to2", logIndex: 1 } as Transfer,
Expand Down Expand Up @@ -92,6 +111,7 @@ describe("LogService", () => {

describe("when transaction details and receipt are defined", () => {
beforeEach(() => {
jest.spyOn(upgradableServiceMock, "getUpgradableAddresses").mockResolvedValueOnce([]);
transactionReceipt = mock<types.TransactionReceipt>({
index: 0,
logs: logs,
Expand Down Expand Up @@ -133,6 +153,9 @@ describe("LogService", () => {
});

describe("when transaction details and receipt are not defined", () => {
beforeEach(() => {
jest.spyOn(upgradableServiceMock, "getUpgradableAddresses").mockResolvedValueOnce([]);
});
it("tracks changed balances", async () => {
await logService.getData(logs, blockDetails);
expect(balanceServiceMock.trackChangedBalances).toHaveBeenCalledTimes(1);
Expand All @@ -146,5 +169,21 @@ describe("LogService", () => {
expect(logsData.transfers).toEqual(transfers);
});
});

describe("when there are upgradable addresses", () => {
beforeEach(() => {
jest.spyOn(upgradableServiceMock, "getUpgradableAddresses").mockResolvedValueOnce(upgradableAddresses);
});
it("returns data with upgradable addresses", async () => {
const logsData = await logService.getData(logs, blockDetails, transactionDetails, transactionReceipt);
expect(upgradableServiceMock.getUpgradableAddresses).toHaveBeenCalledTimes(1);
expect(tokenServiceMock.getERC20Token).toHaveBeenCalledTimes(3);
expect(logsData.tokens).toEqual([
{ l1Address: "l1Address1" },
{ l1Address: "l1Address2" },
{ l2Address: "0xEBf9D3ead9A8c2bb8cEa438B8Dfa9f1AFf44bfa7" },
]);
});
});
});
});
33 changes: 32 additions & 1 deletion packages/data-fetcher/src/log/log.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { TokenService } from "../token/token.service";
import { Transfer } from "../transfer/interfaces/transfer.interface";
import { ContractAddress } from "../address/interface/contractAddress.interface";
import { Token } from "../token/token.service";
import { UpgradableService } from "../upgradable/upgradable.service";

export interface LogsData {
transfers: Transfer[];
Expand All @@ -22,7 +23,8 @@ export class LogService {
private readonly addressService: AddressService,
private readonly balanceService: BalanceService,
private readonly transferService: TransferService,
private readonly tokenService: TokenService
private readonly tokenService: TokenService,
private readonly upgradableService: UpgradableService
) {
this.logger = new Logger(LogService.name);
}
Expand Down Expand Up @@ -60,6 +62,35 @@ export class LogService {
)
).filter((token) => !!token);

this.logger.debug({
message: "Extracting upgradable addresses",
blockNumber: blockDetails.number,
transactionHash,
});

const upgradableAddresses = await this.upgradableService.getUpgradableAddresses(logs, transactionReceipt);
const upgradableTokens = (
await Promise.all(
upgradableAddresses
.filter(
(address) => !contractAddresses.some((contractAddress) => contractAddress.address === address.address)
)
.map(async (upgradableAddress) => {
const proxyAddress = upgradableAddress.address;
const implementationAddress = upgradableAddress.implementationAddress;
const token = await this.tokenService.getERC20Token(
{ ...upgradableAddress, address: implementationAddress },
transactionReceipt
);
return {
...token,
l2Address: proxyAddress,
};
})
)
).filter((token) => !!token);
tokens.push(...upgradableTokens);

logsData.contractAddresses = contractAddresses;
logsData.tokens = tokens;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { types } from "zksync-ethers";
import { mock } from "jest-mock-extended";
import { defaultContractUpgradableHandler } from "./default.handler";

describe("defaultContractUpgradableHandler", () => {
let log: types.Log;
beforeEach(() => {
log = mock<types.Log>({
transactionIndex: 1,
blockNumber: 3233097,
transactionHash: "0x5e018d2a81dbd1ef80ff45171dd241cb10670dcb091e324401ff8f52293841b0",
address: "0x1BEB2aBb1678D8a25431d9728A425455f29d12B7",
topics: [
"0xbc7cd75a20ee27fd9adebab32041f755214dbc6bffa90cc0225b39da2e5c2d3b",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add test examples that test all event types that the handler processes. Currently only 0xbc7cd75a20ee27fd9adebab32041f755214dbc6bffa90cc0225b39da2e5c2d3b is covered. Ideally, I'd use real log examples.

"0x000000000000000000000000a1810a1f32F4DC6c5112b5b837b6975E56b489cc",
],
data: "0x",
index: 8,
blockHash: "0xdfd071dcb9c802f7d11551f4769ca67842041ffb81090c49af7f089c5823f39c",
l1BatchNumber: 604161,
});
});

describe("matches", () => {
it("returns true", () => {
const result = defaultContractUpgradableHandler.matches(log);
expect(result).toBe(true);
});
});

describe("extract", () => {
let transactionReceipt;

beforeEach(() => {
transactionReceipt = mock<types.TransactionReceipt>({
blockNumber: 10,
hash: "transactionHash",
from: "from",
});
});

it("extracts upgraded contract address", () => {
const result = defaultContractUpgradableHandler.extract(log, transactionReceipt);
expect(result.address).toBe("0x1BEB2aBb1678D8a25431d9728A425455f29d12B7");
});

it("extracts block number for the upgraded contract", () => {
const result = defaultContractUpgradableHandler.extract(log, transactionReceipt);
expect(result.blockNumber).toBe(transactionReceipt.blockNumber);
});

it("extracts transaction hash for the upgraded contract", () => {
const result = defaultContractUpgradableHandler.extract(log, transactionReceipt);
expect(result.transactionHash).toBe(transactionReceipt.hash);
});

it("extracts creator address for the upgraded contract", () => {
const result = defaultContractUpgradableHandler.extract(log, transactionReceipt);
expect(result.creatorAddress).toBe(transactionReceipt.from);
});

it("extracts logIndex for the upgraded contract", () => {
const result = defaultContractUpgradableHandler.extract(log, transactionReceipt);
expect(result.logIndex).toBe(log.index);
});

it("extracts implementation address for the upgraded contract", () => {
const result = defaultContractUpgradableHandler.extract(log, transactionReceipt);
expect(result.implementationAddress).toBe("0xa1810a1f32F4DC6c5112b5b837b6975E56b489cc");
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { types } from "zksync-ethers";
import { AbiCoder, keccak256 } from "ethers";
import { ExtractProxyAddressHandler } from "../interface/extractProxyHandler.interface";
import { ProxyAddress } from "../interface/proxyAddress.interface";

const abiCoder: AbiCoder = AbiCoder.defaultAbiCoder();

export const encodedUpgradableEvents = [
"Upgraded(address)",
"BeaconUpgraded(address)",
"OwnershipTransferred(address,address)",
"AdminChanged(address,address)",
"OwnershipTransferred(address,address)",
];
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • OwnershipTransferred(address,address) is listed twice.
  • AdminChanged(address,address) doesn't contain indexed arguments, does it? If so, log.topics[1] will be always null for this event.
  • OwnershipTransferred(address,address) - where the first address is previousOwner and the second one is newOwner, why the previous owner (topics[1]) is used as the implementation address?


const decodedUpgradableEvents = encodedUpgradableEvents.map((event) => `${keccak256(Buffer.from(event)).toString()}`);

export const defaultContractUpgradableHandler: ExtractProxyAddressHandler = {
matches: (log: types.Log): boolean => {
return decodedUpgradableEvents.includes(log.topics[0]);
},
extract: (log: types.Log, txReceipt: types.TransactionReceipt): ProxyAddress => {
if (!log.topics[1]) {
return null;
}

const [address] = abiCoder.decode(["address"], log.topics[1]);
return {
address: log.address,
blockNumber: txReceipt.blockNumber,
transactionHash: txReceipt.hash,
creatorAddress: txReceipt.from,
logIndex: log.index,
implementationAddress: address,
Copy link
Collaborator

Choose a reason for hiding this comment

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

After the latest changes were merged from main, the TS compiler complains that the isEvmLike field is missing for the ProxyAddress type. The exact error can be checked locally or in the failed GitHub Action for this PR.

};
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./default.handler";
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { types } from "zksync-ethers";
import { ProxyAddress } from "./proxyAddress.interface";

export interface ExtractProxyAddressHandler {
matches: (log: types.Log) => boolean;
extract: (log: types.Log, txReceipt: types.TransactionReceipt) => ProxyAddress | null;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { ContractAddress } from "../../address/interface/contractAddress.interface";

export type ProxyAddress = ContractAddress & {
implementationAddress: string;
};
82 changes: 82 additions & 0 deletions packages/data-fetcher/src/upgradable/upgradable.service.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { Test } from "@nestjs/testing";
import { Logger } from "@nestjs/common";
import { mock } from "jest-mock-extended";
import { types } from "zksync-ethers";

import { UpgradableService } from "./upgradable.service";
describe("UpgradableService", () => {
let upgradableService: UpgradableService;

beforeEach(async () => {
const app = await Test.createTestingModule({
providers: [UpgradableService],
}).compile();

app.useLogger(mock<Logger>());

upgradableService = app.get<UpgradableService>(UpgradableService);
});

describe("getUpgradableAddresses", () => {
const logs = [
mock<types.Log>({
topics: [
"0x290afdae231a3fc0bbae8b1af63698b0a1d79b21ad17df0342dfb952fe74f8e5",
"0x000000000000000000000000c7e0220d02d549c4846a6ec31d89c3b670ebe35c",
"0x0100014340e955cbf39159da998b3374bee8f3c0b3c75a7a9e3df6b85052379d",
"0x000000000000000000000000dc187378edd8ed1585fb47549cc5fe633295d571",
],
index: 1,
}),
mock<types.Log>({
topics: [
"0xbc7cd75a20ee27fd9adebab32041f755214dbc6bffa90cc0225b39da2e5c2d3b",
"0x0000000000000000000000000db321efaa9e380d0b37b55b530cdaa62728b9a3",
],
address: "0xdc187378edD8Ed1585fb47549Cc5fe633295d571",
index: 2,
}),
mock<types.Log>({
topics: [
"0x8be0079c531659141344cd1fd0a4f28419497f9722a3daafe3b4186f6b6457e0",
"0x000000000000000000000000481e48ce19781c3ca573967216dee75fdcf70f54",
],
address: "0xD144ca8Aa2E7DFECD56a3CCcBa1cd873c8e5db58",
index: 3,
}),
];

const transactionReceipt = mock<types.TransactionReceipt>({
blockNumber: 10,
hash: "transactionHash",
from: "from",
});

it("returns upgradable addresses", async () => {
const upgradableAddresses = await upgradableService.getUpgradableAddresses(logs, transactionReceipt);
expect(upgradableAddresses).toStrictEqual([
{
address: "0xdc187378edD8Ed1585fb47549Cc5fe633295d571",
implementationAddress: "0x0Db321EFaa9E380d0B37B55B530CDaA62728B9a3",
blockNumber: transactionReceipt.blockNumber,
transactionHash: transactionReceipt.hash,
creatorAddress: transactionReceipt.from,
logIndex: logs[1].index,
},
{
address: "0xD144ca8Aa2E7DFECD56a3CCcBa1cd873c8e5db58",
implementationAddress: "0x481E48Ce19781c3cA573967216deE75FDcF70F54",
blockNumber: transactionReceipt.blockNumber,
transactionHash: transactionReceipt.hash,
creatorAddress: transactionReceipt.from,
logIndex: logs[2].index,
},
]);
});

it("returns an empty array if no logs specified", async () => {
const result = await upgradableService.getUpgradableAddresses(null, transactionReceipt);
expect(result).toStrictEqual([]);
});
});
});
Loading
Loading