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

Fix lag issue with Mailbox message sequence_and_tip #2879

Merged
merged 4 commits into from
Oct 31, 2023

Conversation

tkporter
Copy link
Collaborator

Description

The old implementation of the sequence_and_tip fn in mailbox.rs used to look like this, where it'd end up passing the tip as the lag!

let sequence = match NonZeroU64::new(tip as u64) {
            None => None,
            Some(n) => Some(self.nonce(Some(n)).await?),
        };

Shuffled things around where we correctly just specify the tip as the block number instead of a lag. This means that all functions that were calling wasm_query with the lag needed to start getting the lagged block num instead

Sometimes I believe this would create a bug where we'd ask for a block height 1 and get back RPC errors. I believe most of the time we'd ask for a block height of 0, in which case it'd just act like we were asking for the tip

Drive-by changes

n/a

Related issues

n/a

Backward compatibility

n/a

Testing

plan to roll it out once I get an image

@tkporter tkporter requested a review from daniel-savu as a code owner October 31, 2023 17:58
Comment on lines -140 to -144
let mut client = WasmQueryClient::connect(self.get_conn_url()?).await?;

let mut request = tonic::Request::new(QuerySmartContractStateRequest {
address: self.get_contract_addr()?,
query_data: serde_json::to_string(&payload)?.as_bytes().to_vec(),
Copy link
Contributor

Choose a reason for hiding this comment

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

good spot

@@ -280,37 +282,6 @@ impl CosmosMailboxIndexer {
None
}
}

#[instrument(level = "debug", err, ret, skip(self))]
async fn count(&self, lag: Option<NonZeroU64>) -> ChainResult<u32> {
Copy link
Contributor

Choose a reason for hiding this comment

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

were these not used at all?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no 😛

@tkporter tkporter merged commit 3cf1d56 into trevor/new-featv3-cosmos-oct-28 Oct 31, 2023
2 of 4 checks passed
@tkporter tkporter deleted the trevor/fix-lag branch October 31, 2023 19:12
yorhodes pushed a commit that referenced this pull request Nov 2, 2023
### Description

The old implementation of the `sequence_and_tip` fn in mailbox.rs used
to look like this, where it'd end up passing the tip as the lag!
```
let sequence = match NonZeroU64::new(tip as u64) {
            None => None,
            Some(n) => Some(self.nonce(Some(n)).await?),
        };
```

Shuffled things around where we correctly just specify the `tip` as the
block number instead of a lag. This means that all functions that were
calling `wasm_query` with the lag needed to start getting the lagged
block num instead

Sometimes I believe this would create a bug where we'd ask for a block
height 1 and get back RPC errors. I believe most of the time we'd ask
for a block height of 0, in which case it'd just act like we were asking
for the tip

### Drive-by changes

n/a

### Related issues

n/a

### Backward compatibility

n/a

### Testing

plan to roll it out once I get an image
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants