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

Using fully qualified names in library Transformations as identifiers #1487

Merged
merged 8 commits into from
Jul 24, 2024
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 4 additions & 5 deletions packages/lib-sourcify/src/lib/CheckedContract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -478,11 +478,10 @@ export class CheckedContract {
metadata: this.metadataRaw,
// Sometimes the compiler returns empty object (not falsey). Convert it to undefined (falsey).
immutableReferences:
contract.evm?.deployedBytecode?.immutableReferences &&
Object.keys(contract.evm.deployedBytecode.immutableReferences).length >
0
? contract.evm.deployedBytecode.immutableReferences
: {},
contract.evm?.deployedBytecode?.immutableReferences || {},
creationLinkReferences: contract?.evm?.bytecode?.linkReferences || {},
runtimeLinkReferences:
contract?.evm?.deployedBytecode?.linkReferences || {},
};
}

Expand Down
16 changes: 15 additions & 1 deletion packages/lib-sourcify/src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,25 @@ export interface ImmutableReferences {
start: number;
}>;
}

export interface LinkReferences {
[filePath: string]: {
[libraryName: string]: [
{
length: number;
start: number;
},
];
};
}

export interface RecompilationResult {
creationBytecode: string;
runtimeBytecode: string;
metadata: string;
immutableReferences: ImmutableReferences;
creationLinkReferences: LinkReferences;
runtimeLinkReferences: LinkReferences;
}

export type Transformation = {
Expand All @@ -176,7 +190,7 @@ export type Transformation = {
export const CallProtectionTransformation = (): Transformation => ({
type: 'replace',
reason: 'callProtection',
offset: 0,
offset: 1, // 1 byte is always the PUSH20 opcode 0x73
});

// TransformationValues only has one ConstructorTransformatino so no id field is needed
Expand Down
121 changes: 73 additions & 48 deletions packages/lib-sourcify/src/lib/verification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,18 @@
CallProtectionTransformation,
TransformationValues,
CompiledContractCborAuxdata,
LinkReferences,
} from './types';
import {
decode as bytecodeDecode,
splitAuxdata,
} from '@ethereum-sourcify/bytecode-utils';
import { getAddress, getCreateAddress, keccak256 } from 'ethers';
import {
getAddress,
getCreateAddress,
keccak256,
id as keccak256Str,
} from 'ethers';
import { hexZeroPad, isHexString } from '@ethersproject/bytes';
import { BigNumber } from '@ethersproject/bignumber';
import { defaultAbiCoder as abiCoder, ParamType } from '@ethersproject/abi';
Expand Down Expand Up @@ -114,6 +120,7 @@
runtimeBytecode,
generateRuntimeCborAuxdataPositions,
recompiled.immutableReferences,
recompiled.runtimeLinkReferences,
);
if (match.runtimeMatch === 'partial') {
logDebug('Matched with deployed bytecode', {
Expand All @@ -132,6 +139,7 @@
runtimeBytecode,
generateRuntimeCborAuxdataPositions,
recompiled.immutableReferences,
recompiled.runtimeLinkReferences,
);
},
'runtimeMatch',
Expand Down Expand Up @@ -169,6 +177,7 @@
creatorTxHash,
recompiledMetadata,
generateCreationCborAuxdataPositions,
recompiled.creationLinkReferences,

Check warning on line 180 in packages/lib-sourcify/src/lib/verification.ts

View check run for this annotation

Codecov / codecov/patch

packages/lib-sourcify/src/lib/verification.ts#L180

Added line #L180 was not covered by tests
);
if (match.runtimeMatch === 'partial') {
logDebug('Matched partial with creation tx', {
Expand All @@ -190,6 +199,7 @@
creatorTxHash,
recompiledMetadata,
generateCreationCborAuxdataPositions,
recompiled.creationLinkReferences,

Check warning on line 202 in packages/lib-sourcify/src/lib/verification.ts

View check run for this annotation

Codecov / codecov/patch

packages/lib-sourcify/src/lib/verification.ts#L202

Added line #L202 was not covered by tests
);
},
'creationMatch',
Expand Down Expand Up @@ -413,7 +423,8 @@
recompiledRuntimeBytecode: string,
onchainRuntimeBytecode: string,
generateCborAuxdataPositions: () => Promise<CompiledContractCborAuxdata>,
immutableReferences?: ImmutableReferences,
immutableReferences: ImmutableReferences,
linkReferences: LinkReferences,
) {
// Updating the `match.onchainRuntimeBytecode` here so we are sure to always update it
match.onchainRuntimeBytecode = onchainRuntimeBytecode;
Expand All @@ -436,30 +447,21 @@
);

// Replace the library placeholders in the recompiled bytecode with values from the deployed bytecode
const { replaced, libraryMap } = addLibraryAddresses(
const { replaced, libraryMap } = handleLibraries(
recompiledRuntimeBytecode,
onchainRuntimeBytecode,
linkReferences,
match.runtimeTransformations,
match.runtimeTransformationValues,
);
recompiledRuntimeBytecode = replaced;
if (Object.keys(libraryMap).length > 0) {
// Prepend the library addresses with "0x", this is the format for the DB. FS library-map is without "0x"
match.runtimeTransformationValues.libraries = Object.keys(
libraryMap,
).reduce((libMap: any, lib) => {
libMap[lib] = `0x${libraryMap[lib]}`;
return libMap;
}, {});
}

if (immutableReferences) {
onchainRuntimeBytecode = replaceImmutableReferences(
immutableReferences,
onchainRuntimeBytecode,
match.runtimeTransformations,
match.runtimeTransformationValues,
);
}
onchainRuntimeBytecode = replaceImmutableReferences(
immutableReferences,
onchainRuntimeBytecode,
match.runtimeTransformations,
match.runtimeTransformationValues,
);

// We call generateCborAuxdataPositions before returning because we always need
// to fill cborAuxdata in creation_code_artifacts and runtime_code_artifacts
Expand Down Expand Up @@ -525,6 +527,7 @@
creatorTxHash: string,
recompiledMetadata: Metadata,
generateCborAuxdataPositions: () => Promise<CompiledContractCborAuxdata>,
linkReferences: LinkReferences,
) {
if (recompiledCreationBytecode === '0x') {
match.creationMatch = null;
Expand Down Expand Up @@ -569,24 +572,17 @@
match.creationTransformationValues = {};
}

// The reason why this uses `startsWith` instead of `===` is that creationTxData may contain constructor arguments at the end part.
// Replace the library placeholders in the recompiled bytecode with values from the deployed bytecode
const { replaced, libraryMap } = addLibraryAddresses(
const { replaced, libraryMap } = handleLibraries(
recompiledCreationBytecode,
match.onchainCreationBytecode,
linkReferences,
match.creationTransformations,
match.creationTransformationValues,
);
recompiledCreationBytecode = replaced;
if (Object.keys(libraryMap).length > 0) {
// Prepend the library addresses with "0x", this is the format for the DB. FS library-map is without "0x"
match.creationTransformationValues.libraries = Object.keys(
libraryMap,
).reduce((libMap: any, lib) => {
libMap[lib] = `0x${libraryMap[lib]}`;
return libMap;
}, {});
}

// The reason why this uses `startsWith` instead of `===` is that creationTxData may contain constructor arguments at the end part.
if (match.onchainCreationBytecode.startsWith(recompiledCreationBytecode)) {
// if the bytecode doesn't end with metadata then "partial" match
if (endsWithMetadataHash(recompiledCreationBytecode)) {
Expand Down Expand Up @@ -702,34 +698,62 @@
}
}

export function addLibraryAddresses(
export function handleLibraries(
template: string,
real: string,
linkReferences: LinkReferences,
transformationsArray: Transformation[],
transformationValues: TransformationValues,
): {
replaced: string;
libraryMap: StringMap;
} {
const PLACEHOLDER_START = '__';
const PLACEHOLDER_LENGTH = 40;

const libraryMap: StringMap = {};
for (const file in linkReferences) {
for (const lib in linkReferences[file]) {
for (const linkRefObj of linkReferences[file][lib]) {
const fqn = `${file}:${lib}`; // Fully Qualified (FQ) name

const { start, length } = linkRefObj;
const strStart = start * 2 + 2; // Each byte 2 chars and +2 for 0x
const strLength = length * 2;
const placeholder = template.slice(strStart, strStart + strLength);

// slice(2) removes 0x
const calculatedPlaceholder =
'__$' + keccak256Str(fqn).slice(2).slice(0, 34) + '$__';
// Placeholder format was different pre v0.5.0 https://docs.soliditylang.org/en/v0.4.26/contracts.html#libraries
const trimmedFQN = fqn.slice(0, 36); // in case the fqn is too long
const calculatedPreV050Placeholder = '__' + trimmedFQN.padEnd(38, '_');

if (
!(
placeholder === calculatedPlaceholder ||
placeholder === calculatedPreV050Placeholder

Check warning on line 732 in packages/lib-sourcify/src/lib/verification.ts

View check run for this annotation

Codecov / codecov/patch

packages/lib-sourcify/src/lib/verification.ts#L732

Added line #L732 was not covered by tests
)
)
throw new Error(
`Library placeholder mismatch: ${placeholder} vs ${calculatedPlaceholder} or ${calculatedPreV050Placeholder}`,
);

Check warning on line 737 in packages/lib-sourcify/src/lib/verification.ts

View check run for this annotation

Codecov / codecov/patch

packages/lib-sourcify/src/lib/verification.ts#L736-L737

Added lines #L736 - L737 were not covered by tests

let index = template.indexOf(PLACEHOLDER_START);
while (index !== -1) {
const placeholder = template.slice(index, index + PLACEHOLDER_LENGTH);
const address = real.slice(index, index + PLACEHOLDER_LENGTH);
libraryMap[placeholder] = address;
const address = real.slice(strStart, strStart + strLength);
libraryMap[placeholder] = address;

// Replace regex with simple string replacement
template = template.split(placeholder).join(address);
// Replace the specific occurrence of the placeholder
template =
template.slice(0, strStart) +
address +
template.slice(strStart + strLength);

transformationsArray.push(
// we divide by 2 because we store the length in bytes (without 0x)
LibraryTransformation((index - 2) / 2, placeholder),
);
transformationsArray.push(LibraryTransformation(start, fqn));

index = template.indexOf(PLACEHOLDER_START);
if (!transformationValues.libraries) {
transformationValues.libraries = {};
}
// Prepend the library addresses with "0x", this is the format for the DB. FS library-map is without "0x"
transformationValues.libraries[fqn] = '0x' + address;
}
}
}

return {
Expand All @@ -750,8 +774,9 @@

if (template.startsWith(callProtection)) {
const replacedCallProtection = real.slice(0, 0 + callProtection.length);
const callProtectionAddress = replacedCallProtection.slice(4); // remove 0x73
transformationsArray.push(CallProtectionTransformation());
transformationValues.callProtection = replacedCallProtection;
transformationValues.callProtection = '0x' + callProtectionAddress;

return replacedCallProtection + template.substring(callProtection.length);
}
Expand Down
5 changes: 5 additions & 0 deletions packages/lib-sourcify/test/verification.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,7 @@ describe('lib-sourcify tests', () => {
wrongCreatorTxHash,
recompiledMetadata,
generateCreationCborAuxdataPositions,
recompiled.creationLinkReferences,
);
} catch (err) {
if (err instanceof Error) {
Expand Down Expand Up @@ -772,6 +773,7 @@ describe('lib-sourcify tests', () => {
txHash,
recompiledMetadata,
generateCreationCborAuxdataPositions,
recompiled.creationLinkReferences,
);
expectMatch(match, null, contractAddress, undefined); // status is null
});
Expand Down Expand Up @@ -817,6 +819,7 @@ describe('lib-sourcify tests', () => {
creatorTxHash,
recompiledMetadata,
generateCreationCborAuxdataPositions,
recompiled.creationLinkReferences,
);
expectMatch(match, null, contractAddress, undefined); // status is null
});
Expand Down Expand Up @@ -854,6 +857,7 @@ describe('lib-sourcify tests', () => {
creatorTxHash,
recompiledMetadata,
generateCreationCborAuxdataPositions,
recompiled.creationLinkReferences,
);
try {
expect(match.creationMatch).to.equal('perfect');
Expand Down Expand Up @@ -897,6 +901,7 @@ describe('lib-sourcify tests', () => {
creatorTxHash,
recompiledMetadata,
generateCreationCborAuxdataPositions,
recompiled.creationLinkReferences,
);
try {
expect(match.creationMatch).to.equal('partial');
Expand Down
1 change: 1 addition & 0 deletions services/server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
"json-refs": "3.0.15",
"memorystore": "1.6.7",
"pg": "8.12.0",
"semver": "7.6.2",
"serve-index": "1.9.1",
"solc": "0.8.26",
"swagger-ui-express": "5.0.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Match,
CheckedContract,
Status,
StringMap,
} from "@ethereum-sourcify/lib-sourcify";
import logger from "../../../common/logger";
import * as Database from "../utils/database-util";
Expand All @@ -22,11 +23,12 @@
PaginatedContractData,
} from "../../types";
import config from "config";
import Path from "path";
import Path, { format } from "path";
import { getFileRelativePath } from "../utils/util";
import { getAddress } from "ethers";
import { getAddress, id as keccak256Str } from "ethers";
import { BadRequestError } from "../../../common/errors";
import { RWStorageIdentifiers } from "./identifiers";
import semver from "semver";

const MAX_RETURNED_CONTRACTS_BY_GETCONTRACTS = 200;

Expand Down Expand Up @@ -307,9 +309,25 @@
sourcifyMatch?.runtime_values?.libraries &&
Object.keys(sourcifyMatch.runtime_values.libraries).length > 0
) {
files["library-map.json"] = JSON.stringify(
// Must convert "contracts/file.sol:MyLib" FQN format to the placeholder format __$keccak256(file.sol:MyLib)$___ or __MyLib__________
const formattedLibraries: StringMap = {};
for (const [key, value] of Object.entries(
sourcifyMatch.runtime_values.libraries,
);
)) {
let formattedKey;
// Solidity >= 0.5.0 is __$keccak256(file.sol:MyLib)$__ (total 40 characters)
if (semver.gte(sourcifyMatch.metadata.compiler.version, "0.5.0")) {
formattedKey =
"__$" + keccak256Str(key).slice(2).slice(0, 34) + "$__";
} else {
// Solidity < 0.5.0 is __MyLib__________ (total 40 characters)
kuzdogan marked this conversation as resolved.
Show resolved Hide resolved
const libName = key.split(":")[1];
const trimmedLibName = libName.slice(0, 36); // in case it's longer
formattedKey = "__" + trimmedLibName.padEnd(38, "_");
}

Check warning on line 327 in services/server/src/server/services/storageServices/SourcifyDatabaseService.ts

View check run for this annotation

Codecov / codecov/patch

services/server/src/server/services/storageServices/SourcifyDatabaseService.ts#L323-L327

Added lines #L323 - L327 were not covered by tests
Comment on lines +323 to +327
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 24, 2024

Choose a reason for hiding this comment

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

Add test coverage for library names handling for Solidity < 0.5.0.

The logic for handling library names for Solidity < 0.5.0 is not covered by tests.

Do you want me to generate the unit testing code or open a GitHub issue to track this task?

Tools
GitHub Check: codecov/patch

[warning] 323-327: services/server/src/server/services/storageServices/SourcifyDatabaseService.ts#L323-L327
Added lines #L323 - L327 were not covered by tests

Copy link
Member Author

Choose a reason for hiding this comment

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

Generate unit testing code

Copy link
Contributor

Choose a reason for hiding this comment

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

@kuzdogan, here are the unit tests to cover the handling of library names for Solidity < 0.5.0 in the SourcifyDatabaseService class:

import { SourcifyDatabaseService } from '../src/server/services/storageServices/SourcifyDatabaseService';
import { StorageService } from '../src/server/services/StorageService';
import { Pool } from 'pg';
import { Match } from '@ethereum-sourcify/lib-sourcify';
import semver from 'semver';

describe('SourcifyDatabaseService', () => {
  let sourcifyDatabaseService: SourcifyDatabaseService;
  let mockStorageService: jest.Mocked<StorageService>;
  let mockDatabasePool: jest.Mocked<Pool>;

  beforeEach(() => {
    mockStorageService = {} as jest.Mocked<StorageService>;
    mockDatabasePool = {} as jest.Mocked<Pool>;
    sourcifyDatabaseService = new SourcifyDatabaseService(mockStorageService, { pool: mockDatabasePool });
  });

  describe('getFiles', () => {
    it('should format library names correctly for Solidity < 0.5.0', async () => {
      const chainId = '1';
      const address = '0x1234567890abcdef1234567890abcdef12345678';
      const sourcifyMatch: Match = {
        address,
        chainId,
        runtimeMatch: 'partial',
        creationMatch: 'partial',
        storageTimestamp: new Date(),
        runtime_values: {
          libraries: {
            'contracts/MyLib.sol:MyLib': '0xabcdefabcdefabcdefabcdefabcdefabcdefabcdef'
          }
        },
        metadata: {
          compiler: {
            version: '0.4.26'
          }
        }
      };

      jest.spyOn(Database, 'getSourcifyMatchByChainAddress').mockResolvedValueOnce({
        rowCount: 1,
        rows: [sourcifyMatch]
      });

      const result = await sourcifyDatabaseService.getFiles(chainId, address);

      expect(result.files['library-map.json']).toBeDefined();
      const formattedLibraries = JSON.parse(result.files['library-map.json']);
      expect(formattedLibraries['__MyLib___________________________']).toBe('0xabcdefabcdefabcdefabcdefabcdefabcdefabcdef');
    });

    it('should format library names correctly for Solidity >= 0.5.0', async () => {
      const chainId = '1';
      const address = '0x1234567890abcdef1234567890abcdef12345678';
      const sourcifyMatch: Match = {
        address,
        chainId,
        runtimeMatch: 'partial',
        creationMatch: 'partial',
        storageTimestamp: new Date(),
        runtime_values: {
          libraries: {
            'contracts/MyLib.sol:MyLib': '0xabcdefabcdefabcdefabcdefabcdefabcdefabcdef'
          }
        },
        metadata: {
          compiler: {
            version: '0.5.0'
          }
        }
      };

      jest.spyOn(Database, 'getSourcifyMatchByChainAddress').mockResolvedValueOnce({
        rowCount: 1,
        rows: [sourcifyMatch]
      });

      const result = await sourcifyDatabaseService.getFiles(chainId, address);

      expect(result.files['library-map.json']).toBeDefined();
      const formattedLibraries = JSON.parse(result.files['library-map.json']);
      expect(formattedLibraries['__$' + keccak256Str('contracts/MyLib.sol:MyLib').slice(2).slice(0, 34) + '$__']).toBe('0xabcdefabcdefabcdefabcdefabcdefabcdefabcdef');
    });
  });
});

These tests cover the logic for handling library names for both Solidity < 0.5.0 and Solidity >= 0.5.0. The tests ensure that the library names are correctly formatted to their respective placeholder formats.


Learnings added
Learnt from: kuzdogan
PR: ethereum/sourcify#1487
File: services/server/src/server/services/storageServices/SourcifyDatabaseService.ts:323-327
Timestamp: 2024-07-24T13:35:59.966Z
Learning: Generate unit tests for handling library names in Solidity < 0.5.0 by creating scenarios where the library names are processed and ensuring that the output matches the expected placeholder format.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do it in nock, chai and mocha

Copy link
Contributor

Choose a reason for hiding this comment

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

@kuzdogan, here are the unit tests rewritten to use nock, chai, and mocha:

import { expect } from 'chai';
import nock from 'nock';
import { SourcifyDatabaseService } from '../src/server/services/storageServices/SourcifyDatabaseService';
import { StorageService } from '../src/server/services/StorageService';
import { Pool } from 'pg';
import { Match } from '@ethereum-sourcify/lib-sourcify';
import semver from 'semver';
import * as Database from '../src/server/services/utils/database-util';

describe('SourcifyDatabaseService', () => {
  let sourcifyDatabaseService: SourcifyDatabaseService;
  let mockStorageService: StorageService;
  let mockDatabasePool: Pool;

  beforeEach(() => {
    mockStorageService = {} as StorageService;
    mockDatabasePool = {} as Pool;
    sourcifyDatabaseService = new SourcifyDatabaseService(mockStorageService, { pool: mockDatabasePool });
  });

  describe('getFiles', () => {
    it('should format library names correctly for Solidity < 0.5.0', async () => {
      const chainId = '1';
      const address = '0x1234567890abcdef1234567890abcdef12345678';
      const sourcifyMatch: Match = {
        address,
        chainId,
        runtimeMatch: 'partial',
        creationMatch: 'partial',
        storageTimestamp: new Date(),
        runtime_values: {
          libraries: {
            'contracts/MyLib.sol:MyLib': '0xabcdefabcdefabcdefabcdefabcdefabcdefabcdef'
          }
        },
        metadata: {
          compiler: {
            version: '0.4.26'
          }
        }
      };

      nock('http://localhost')
        .get(`/sourcifyMatch/${chainId}/${address}`)
        .reply(200, {
          rowCount: 1,
          rows: [sourcifyMatch]
        });

      const result = await sourcifyDatabaseService.getFiles(chainId, address);

      expect(result.files['library-map.json']).to.exist;
      const formattedLibraries = JSON.parse(result.files['library-map.json']);
      expect(formattedLibraries['__MyLib___________________________']).to.equal('0xabcdefabcdefabcdefabcdefabcdefabcdefabcdef');
    });

    it('should format library names correctly for Solidity >= 0.5.0', async () => {
      const chainId = '1';
      const address = '0x1234567890abcdef1234567890abcdef12345678';
      const sourcifyMatch: Match = {
        address,
        chainId,
        runtimeMatch: 'partial',
        creationMatch: 'partial',
        storageTimestamp: new Date(),
        runtime_values: {
          libraries: {
            'contracts/MyLib.sol:MyLib': '0xabcdefabcdefabcdefabcdefabcdefabcdefabcdef'
          }
        },
        metadata: {
          compiler: {
            version: '0.5.0'
          }
        }
      };

      nock('http://localhost')
        .get(`/sourcifyMatch/${chainId}/${address}`)
        .reply(200, {
          rowCount: 1,
          rows: [sourcifyMatch]
        });

      const result = await sourcifyDatabaseService.getFiles(chainId, address);

      expect(result.files['library-map.json']).to.exist;
      const formattedLibraries = JSON.parse(result.files['library-map.json']);
      expect(formattedLibraries['__$' + keccak256Str('contracts/MyLib.sol:MyLib').slice(2).slice(0, 34) + '$__']).to.equal('0xabcdefabcdefabcdefabcdefabcdefabcdefabcdef');
    });
  });
});

These tests now use nock to mock HTTP requests, chai for assertions, and mocha as the test runner. This should align with your preferred testing stack.


Learnings used
Learnt from: kuzdogan
PR: ethereum/sourcify#1487
File: services/server/src/server/services/storageServices/SourcifyDatabaseService.ts:323-327
Timestamp: 2024-07-24T13:36:00.611Z
Learning: Generate unit tests for handling library names in Solidity < 0.5.0 by creating scenarios where the library names are processed and ensuring that the output matches the expected placeholder format.

formattedLibraries[formattedKey] = value;
}
files["library-map.json"] = JSON.stringify(formattedLibraries);
}

if (
Expand Down
Loading