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

Mailbox indexer #39

Merged
merged 23 commits into from
Mar 23, 2023
Merged

Mailbox indexer #39

merged 23 commits into from
Mar 23, 2023

Conversation

tkporter
Copy link
Collaborator

@tkporter tkporter commented Jan 6, 2023

  • Moves to the recommended repo structure for an indexer project: https://fuellabs.github.io/fuel-indexer/v0.1.12/getting-started/fuel-indexer-project.html#with-a-fuel-project
  • Adds log_with_id to bytes_extended, which is used for logging dispatched messages. The rationale is that logs generated by forc compiler make use of the rB register (see https://fuellabs.github.io/fuel-specs/master/vm/instruction_set.html#logd-log-data-event) to have a marker value so that logs can be identified / differentiated. The compiler will look at all logs and generate marker values starting with 0 and counting upward, and it generates the corresponding log/logd instructions with these marker values. You can see these are also present in the ABI if you look at the loggedTypes section. Because we're calling the logd instruction directly in an asm block, we don't get the luxury of the compiler auto-generating a marker value to easily identify this log, so we have to do it manually. This is imo the easiest way to allow us to easily identify dispatched message logs. Tbh I'd be open to other options here, either in this PR or in the future when we revisit Fuel with greater effort closer to their mainnet date
  • Adds indexer/mailbox, which so far indexes dispatched messages. There are a number of necessary quirks that I've done my best to annotate with comments & relevant issues. I recommend reading the README.md in there first and referring to the Fuel Indexer Book / docs as needed.
  • Adds the deploy package, adapted from Super basic deploy tooling #15. It now allows messages to be sent
  • Drive-by update to forc to 0.35.5, some cargo fmt, and cargo clippy changes

@tkporter tkporter requested a review from yorhodes January 6, 2023 12:07
@ControlCplusControlV
Copy link

If you add this to your .gitattributes it will add syntax highlighting on Github btw

Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

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

couldn't get deployment working so have much still to test but great work and docs!

const expectedMailboxContractId = ContractUtils.getContractId(
bytecode,
CONTRACT_SALT,
ContractUtils.getContractStorageRoot([]),
Copy link
Member

Choose a reason for hiding this comment

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

why is ID derived from storage root?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Check out https://specs.fuel.network/master/protocol/id/contract.html#contract-id, it's just part of the way they implemented computing IDs when a contract is deployed

Comment on lines +80 to +91
if (maybeDeployedContract) {
console.log('Contract already deployed');
return new Contract(
expectedMailboxContractId,
HyperlaneMailboxAbi__factory.abi,
wallet,
);
}

console.log('Deploying contract...');
return factory.deployContract({
salt: CONTRACT_SALT,
Copy link
Member

Choose a reason for hiding this comment

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

does factory.deployContract fail if already deployed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah it does, you get something like this:

Error: ClientError: Transaction(0x74deef4b669dfca1d2a5e1679639c29ea53e572268fa123e7359286f85c9711d) execution error: Panic(ContractIdAlreadyDeployed)

indexer/mailbox/README.md Outdated Show resolved Hide resolved
deploy/README.md Outdated Show resolved Hide resolved
In another terminal, build and deploy the Mailbox. This is idempotent if just deploying the mailbox.

```
yarn deploy
Copy link
Member

Choose a reason for hiding this comment

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

Screen Shot 2023-03-20 at 5 38 58 PM

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

did you have yarn local-node running in a different terminal?

Copy link
Member

Choose a reason for hiding this comment

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

yes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmmmm interesting
I assume you did a fresh yarn in the deploy package?

Copy link
Member

Choose a reason for hiding this comment

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

caused by node v19

type DispatchedMessage {
# The nonce as a u64
# See https://fuellabs.github.io/fuel-indexer/master/components/database/ids.html
id: ID!
Copy link
Member

Choose a reason for hiding this comment

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

easily conflated with contract_id

Comment on lines +34 to +35
// See https://fuellabs.github.io/fuel-indexer/v0.1.12/components/database/ids.html
id: message.nonce as u64,
Copy link
Member

Choose a reason for hiding this comment

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

ah nvm about the id comment then

Comment on lines +49 to +55
contract_id: log_metadata.contract_id,
block_number: log_metadata.block_number,
block_hash: log_metadata.block_hash,
transaction_hash: log_metadata.transaction_hash,
transaction_index: log_metadata.transaction_index,
receipt_index: log_metadata.receipt_index,
}
Copy link
Member

Choose a reason for hiding this comment

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

is there no spread operator?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

unfortunately no
The closest is you can do:

MyStruct {
  foo,
  ..my_existing_struct
}

but my_existing_struct has to be an instance of the MyStruct struct

indexer/mailbox/src/message.rs Outdated Show resolved Hide resolved
impl HyperlaneMessage {
/// Convert the message to a message id
pub fn id(&self) -> H256 {
H256::from_slice(Keccak256::new().chain(self.to_vec()).finalize().as_slice())
Copy link
Member

Choose a reason for hiding this comment

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

bit hard to read

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also copied directly from the monorepo so I'm inclined to leave as is

But what I can do instead is do what I mention here https://github.com/hyperlane-xyz/fuel-contracts/pull/39/files/ec0ceaccbbbd620dbb78207bb1d2735c98a0f287#diff-c1df921b29777e810d4612bf6e315c71cf5f404d83cf0413cdeea3b4d2134aceR10 and move these into their own crate in the monorepo so that we don't need to copy and paste these

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

actually this may be a decent undertaking so I'd like to not block this PR on that. I created hyperlane-xyz/hyperlane-monorepo#1977 to do this later


message_id: Bytes32!

# TODO: rip the following out into a LogMetadata type.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tracking this with #50

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