Add logic for writing signatures to db inside `SourcifyDatabaseService` #2357

manuelwedler · 2025-09-04T14:06:37Z

adds a new signature-util.ts to extract signatures from the ABI via ethers
writes the signatures to the new table in a separate db transaction, such that verification job will still succeed even though there is an error while writing the signatures
storeSignatures is also called when a match is updated, because it cannot write any duplicates due to the primary key, nor raise errors due to the ON CONFLICT clause

kuzdogan

Nice and simple. Just a few comments

kuzdogan · 2025-09-04T15:25:13Z

services/server/src/server/services/utils/signature-util.ts

+        signatures.push(getSignatureData(fragment));
+    }


We should wrapt this in a try/catch because fragment.format() will fail for some library contracts that have an invalid "type" field. Library ABI items are allowed to have custom types because their ABIs are not intended to be publicly callable. However this breaks with ethers when assembling the signature. See the latest messages in the Solidity private chat.

Let's also add a test case for the above issue

Thanks for pointing this out. I made the function throwing in this case now, so we would not store any of the signatures if only one custom type was found. Or was your intention rather to ignore the fragments with custom types?

kuzdogan · 2025-09-04T15:35:04Z

services/server/src/server/services/storageServices/SourcifyDatabaseService.ts

    try {
      const { type, verifiedContractId, oldVerifiedContractId } =
-        await super.insertOrUpdateVerification(verification, poolClient);
+        await this.insertOrUpdateVerification(verification, poolClient);


Why the change? Does it have an effect on semantics?

good catch. super can actually stay. I think it's a leftover from Claude which implemented the feature in a different way at first.

kuzdogan · 2025-09-04T15:48:07Z

services/server/src/server/services/storageServices/SourcifyDatabaseService.ts

+      const signatureColumns = signatureData.map((sig) => ({
+        signature_hash_32: bytesFromString<BytesKeccak>(sig.signatureHash32),
+        signature: sig.signature,
+        signature_type: sig.signatureType,
+      }));
+
+      await this.database.insertSignatures(signatureColumns, poolClient);
+      await this.database.insertCompiledContractSignatures(
+        compilationId,


Small thing but we are passing the same signatureColumns to both insertSignatures and insertCompiledContractSignatures functions. Even though the parameter signatures in both functions have different types. This works because the signatureColumns is a union of both types.

I don't have a strong opinion here but just wanted to point out.

Yes, I did this on purpose to keep the code shorter. IMO, when you accept an object typed via an interface, you should also expect the object to possibly have more properties, because TypeScript doesn't guarantee the absence of other properties ("duck typing").

kuzdogan · 2025-09-04T15:56:53Z

services/server/src/server/services/VerificationService.ts

      | VerifyFromMetadataInput
      | VerifyFromEtherscanInput,
-  ): Promise<void> {
+  ): void {


Good catch, how did you recognize this?

Actually this makes me realize now this function is difficult to read with .then()s. It's ok though.

I just read over the function and wondered why the function is async.

We mainly have the then() style here because we need to keep a reference to the promise object. We could move the inner logic to another function to use await, but I think it's fine.

kuzdogan · 2025-09-04T15:58:49Z

services/server/test/integration/storageServices/SourcifyDatabaseService.spec.ts

+    expect(storeSignature).to.exist;
+
+    const expectedRetrieveSignatureHash32 = bytesFromString(
+      keccak256str(retrieveSignature!.signature),


Isn't this self referencing? This should be calculated from teh MockVerificationExport or raw strings above?

kuzdogan · 2025-09-08T12:24:01Z

services/server/src/server/services/utils/signature-util.ts

+  } catch (error) {
+    throw new Error(
+      "Failed to extract signatures from ABI due to an invalid fragment",
+    );
+  }


This should not throw, or if it does I don't see it being caught anywhere. If there's an invalid type e.g. when it's a library contract ABI we should simply skip. Right now this throws here

sourcify/services/server/src/server/services/storageServices/SourcifyDatabaseService.ts

Lines 913 to 914 in 6bfbf5f

const signatureData = extractSignaturesFromAbi(abi);

const signatureColumns = signatureData.map((sig) => ({

I still didn't understand precisely how you want to handle this case. If it is a library contract, do you want to skip storing all signatures of the ABI, or do you want to just skip the ABI fragments with custom types and still store the other signatures? Both options seem reasonable to me.

The storeSignatures function actually has a try-catch which is wrapping the lines you referenced here.

Hmm I'd say just skip the ABI fragments that are invalid ie. not an "error", "function", or "event". This would also include the constructors. So if a library has valid signatures we will save them.

From my understanding, as is, this code would break at the invalid signature and won't save the rest of the valid signatures. Ie if a contract has signatures of type:

function

InvalidType

function

~~it will break at 2. and won't save 3., no?~~

Edit: actually it will break at 2. and won't save any of the signatures. I think we should save the valid ones

We can just silently skip processing if the fragment.type is not one of the valid ones and won't enter it to the getSignatureData which will throw via ethersjs.

Alright sounds good to me. I just think we should ignore inside the getSignatureData function and not throw from it in any case then.

manuelwedler added 3 commits September 4, 2025 11:14

Add signature-util for extracting SignatureData from ABIs

4b327c1

Add signature tables interfaces

fd4bd64

Add logic for writing signatures to SourcifyDatabaseService

9be3c13

github-project-automation bot added this to Sourcify Public Sep 4, 2025

github-project-automation bot moved this to Triage in Sourcify Public Sep 4, 2025

Add tests

3702f3b

manuelwedler force-pushed the 4bytes-write-logic branch from 36244b2 to 3702f3b Compare September 4, 2025 14:07

kuzdogan reviewed Sep 4, 2025

View reviewed changes

address PR feedback

6bfbf5f

manuelwedler requested a review from kuzdogan September 8, 2025 08:49

kuzdogan requested changes Sep 8, 2025

View reviewed changes

Ignore ABI fragments with custom type

c4e7116

kuzdogan approved these changes Sep 8, 2025

View reviewed changes

manuelwedler merged commit 3fa5023 into staging Sep 8, 2025
5 checks passed

github-project-automation bot moved this from Triage to Sprint - Done in Sourcify Public Sep 8, 2025

manuelwedler deleted the 4bytes-write-logic branch September 8, 2025 14:36

marcocastignoli moved this from Sprint - Done to COMPLETED in Sourcify Public Sep 11, 2025

	const signatureData = extractSignaturesFromAbi(abi);
	const signatureColumns = signatureData.map((sig) => ({

Add logic for writing signatures to db inside SourcifyDatabaseService #2357

Add logic for writing signatures to db inside SourcifyDatabaseService #2357

Uh oh!

Conversation

manuelwedler commented Sep 4, 2025

Uh oh!

kuzdogan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kuzdogan Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add logic for writing signatures to db inside `SourcifyDatabaseService` #2357

Add logic for writing signatures to db inside `SourcifyDatabaseService` #2357

kuzdogan Sep 8, 2025 •

edited

Loading