-
Notifications
You must be signed in to change notification settings - Fork 114
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/factory supports parameter within struct #1343
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
export const ChildContractAbi = [ | ||
{ | ||
inputs: [ | ||
{ internalType: "string", name: "_name", type: "string" }, | ||
{ internalType: "uint256", name: "_initialValue", type: "uint256" }, | ||
], | ||
stateMutability: "nonpayable", | ||
type: "constructor", | ||
}, | ||
{ | ||
anonymous: false, | ||
inputs: [ | ||
{ | ||
indexed: true, | ||
internalType: "address", | ||
name: "child", | ||
type: "address", | ||
}, | ||
{ | ||
indexed: true, | ||
internalType: "address", | ||
name: "updater", | ||
type: "address", | ||
}, | ||
{ | ||
indexed: false, | ||
internalType: "uint256", | ||
name: "oldValue", | ||
type: "uint256", | ||
}, | ||
{ | ||
indexed: false, | ||
internalType: "uint256", | ||
name: "newValue", | ||
type: "uint256", | ||
}, | ||
], | ||
name: "ValueUpdated", | ||
type: "event", | ||
}, | ||
{ | ||
inputs: [], | ||
name: "factory", | ||
outputs: [{ internalType: "address", name: "", type: "address" }], | ||
stateMutability: "view", | ||
type: "function", | ||
}, | ||
{ | ||
inputs: [{ internalType: "uint256", name: "_newValue", type: "uint256" }], | ||
name: "setValue", | ||
outputs: [], | ||
stateMutability: "nonpayable", | ||
type: "function", | ||
}, | ||
{ | ||
inputs: [], | ||
name: "value", | ||
outputs: [{ internalType: "uint256", name: "", type: "uint256" }], | ||
stateMutability: "view", | ||
type: "function", | ||
}, | ||
] as const; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,21 @@ const llamaFactoryEventAbiItem = parseAbiItem( | |
"event LlamaInstanceCreated(address indexed deployer, string indexed name, address llamaCore, address llamaExecutor, address llamaPolicy, uint256 chainId)", | ||
); | ||
|
||
const FactoryEventSimpleParamsAbiItem = parseAbiItem([ | ||
"event CreateMarket(bytes32 indexed id ,MarketParams marketParams)", | ||
"struct MarketParams {address loanToken; address collateralToken; address oracle; address irm; uint256 lltv;}", | ||
]); | ||
|
||
const FactoryEventWithDynamicChildParamsAbiItem = parseAbiItem([ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also include a test case where a struct/array is one of the indexed arguments and the user attempts to use one of its properties as the child address parameter. I think we'll have to validate against this - the address is not actually available in the event body because the value gets hashed and then stored as one of the topics. |
||
"event ChildCreated(address indexed creator, ChildInfo child, uint256 indexed timestamp)", | ||
"struct ChildInfo { address childAddress; string name; uint256 initialValue; uint256 creationTime; address creator; }", | ||
]); | ||
|
||
const FactoryEventWithDynamicChildParamsAbiItem2 = parseAbiItem([ | ||
"event ChildCreated(address creator, ChildInfo child, uint256 timestamp)", | ||
"struct ChildInfo { address childAddress; string name; uint256 initialValue; uint256 creationTime; address creator; }", | ||
]); | ||
|
||
test("buildLogFactory throws if provided parameter not found in inputs", () => { | ||
expect(() => | ||
buildLogFactory({ | ||
|
@@ -48,3 +63,61 @@ test("buildLogFactory handles LlamaInstanceCreated llamaPolicy", () => { | |
childAddressLocation: "offset64", | ||
}); | ||
}); | ||
|
||
test("buildLogFactory throws if provided parameter not found in inputs", () => { | ||
expect(() => | ||
buildLogFactory({ | ||
address: "0xa", | ||
event: FactoryEventSimpleParamsAbiItem, | ||
parameter: "marketParams.fake", | ||
chainId: 1, | ||
}), | ||
).toThrowError( | ||
"Factory event parameter not found in factory event signature. Got 'fake', expected one of ['loanToken', 'collateralToken', 'oracle', 'irm', 'lltv'].", | ||
); | ||
}); | ||
|
||
test("buildLogFactory handles CreateMarket", () => { | ||
const criteria = buildLogFactory({ | ||
address: "0xa", | ||
event: FactoryEventSimpleParamsAbiItem, | ||
parameter: "marketParams.oracle", | ||
chainId: 1, | ||
}); | ||
|
||
expect(criteria).toMatchObject({ | ||
address: "0xa", | ||
eventSelector: getEventSelector(FactoryEventSimpleParamsAbiItem), | ||
childAddressLocation: "offset64", | ||
}); | ||
}); | ||
|
||
test("buildLogFactory handles ChildCreated", () => { | ||
const criteria = buildLogFactory({ | ||
address: "0xa", | ||
event: FactoryEventWithDynamicChildParamsAbiItem, | ||
parameter: "child.childAddress", | ||
chainId: 1, | ||
}); | ||
|
||
expect(criteria).toMatchObject({ | ||
address: "0xa", | ||
eventSelector: getEventSelector(FactoryEventWithDynamicChildParamsAbiItem), | ||
childAddressLocation: "offset32", | ||
}); | ||
}); | ||
|
||
test("buildLogFactory handles ChildCreated", () => { | ||
const criteria = buildLogFactory({ | ||
address: "0xa", | ||
event: FactoryEventWithDynamicChildParamsAbiItem2, | ||
parameter: "child.childAddress", | ||
chainId: 1, | ||
}); | ||
|
||
expect(criteria).toMatchObject({ | ||
address: "0xa", | ||
eventSelector: getEventSelector(FactoryEventWithDynamicChildParamsAbiItem2), | ||
childAddressLocation: "offset96", | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import type { LogFactory } from "@/sync/source.js"; | ||
import { toLowerCase } from "@/utils/lowercase.js"; | ||
import { getBytesConsumedByParam } from "@/utils/offset.js"; | ||
import { getBytesConsumedByParam, hasDynamicChild } from "@/utils/offset.js"; | ||
import type { AbiEvent } from "abitype"; | ||
import { type Address, getEventSelector } from "viem"; | ||
|
||
|
@@ -15,6 +15,12 @@ export function buildLogFactory({ | |
parameter: string; | ||
chainId: number; | ||
}): LogFactory { | ||
const parameterParts = parameter.split("."); | ||
|
||
let offset = 0; | ||
|
||
parameter = parameterParts[0]!; | ||
|
||
Comment on lines
+18
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should be an explicit validation check that this exists, like Also, I think these lines should be below where we build the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the check, but I think these lines should be before building the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough, we just generally try to group concerns for readability, eg first parse address, then parse selector, then event, then parameter. Another nit - we avoid re-assigning function parameters like you're doing with const firstParameterSegment = parameterSegments[0]; |
||
const address = Array.isArray(_address) | ||
? _address.map(toLowerCase) | ||
: toLowerCase(_address); | ||
|
@@ -51,11 +57,46 @@ export function buildLogFactory({ | |
); | ||
} | ||
|
||
let offset = 0; | ||
for (let i = 0; i < nonIndexedInputPosition; i++) { | ||
offset += getBytesConsumedByParam(nonIndexedInputs[i]!); | ||
} | ||
|
||
let prvInput = nonIndexedInputs[nonIndexedInputPosition]!; | ||
|
||
for (let i = 1; i < parameterParts.length; i++) { | ||
if (!("components" in prvInput)) { | ||
throw new Error(`Parameter ${parameter} is not a tuple or struct type`); | ||
} | ||
|
||
const dynamicChildFlag = hasDynamicChild(prvInput); | ||
|
||
if (dynamicChildFlag) { | ||
for (let j = nonIndexedInputPosition; j < nonIndexedInputs.length; j++) { | ||
// bytes consumed by successor siblings after the current one | ||
offset += getBytesConsumedByParam(nonIndexedInputs[j]!); | ||
} | ||
} | ||
|
||
const components = prvInput.components; | ||
|
||
parameter = parameterParts[i]!; | ||
|
||
const inputIndex = components.findIndex( | ||
(input) => input.name === parameter, | ||
); | ||
if (inputIndex === -1) { | ||
throw new Error( | ||
`Factory event parameter not found in factory event signature. Got '${parameter}', expected one of [${components | ||
.map((i) => `'${i.name}'`) | ||
.join(", ")}].`, | ||
); | ||
} | ||
for (let j = 0; j < inputIndex; j++) { | ||
offset += getBytesConsumedByParam(components[j]!); | ||
} | ||
prvInput = components[inputIndex]!; | ||
} | ||
|
||
return { | ||
type: "log", | ||
chainId, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parseAbi
here because there are multiple itemscamelCase
for most identifiersThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.