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.

Sign up for GitHub

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

add pre_validate #109

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

add pre_validate #109

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8028521
add pre_validate
Art3miX Nov 24, 2024
b540f37
fix the leak
Art3miX Nov 25, 2024
ab7fb62
fmt
Art3miX Nov 25, 2024
29c659a
add missing libraries
Art3miX Nov 25, 2024
9f98a42
merge main
Art3miX Nov 25, 2024
208ed66
merge main
Art3miX Nov 26, 2024
61f13f3
update schema
Art3miX Nov 26, 2024
66dad55
Merge branch 'main' into art3mix/manager-pre-validate
Art3miX Nov 26, 2024
581f8e3
fix
Art3miX Nov 26, 2024
eff3e19
remove the leak and copy the mockApi
Art3miX Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock
View file
Open in desktop

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

1 change: 0 additions & 1 deletion local-interchaintest/examples/manager_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ fn main() -> Result<(), Box<dyn Error>> {
&authorization_addr,
format!("update_library_{}", library_1.get_library_id()).as_str(),
);

contract_execute(
test_ctx
.get_request_builder()
Expand Down
12 changes: 6 additions & 6 deletions packages/valence-macros/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ pub fn manager_impl_library_configs(_attr: TokenStream, input: TokenStream) -> T
let mut update_msg_matches = Vec::new();
let mut replace_config_matches = Vec::new();
let mut get_instantiate_msg_matches = Vec::new();
let mut per_validate_matches = Vec::new();
let mut pre_validate_matches = Vec::new();
let mut get_account_ids_matches = Vec::new();

for variant in variants {
Expand All @@ -181,7 +181,7 @@ pub fn manager_impl_library_configs(_attr: TokenStream, input: TokenStream) -> T
get_instantiate_msg_matches.push(quote! {
#enum_ident::None => return Err(LibraryError::NoLibraryConfig)
});
per_validate_matches.push(quote! {
pre_validate_matches.push(quote! {
#enum_ident::None => Err(LibraryError::NoLibraryConfig)
});
get_account_ids_matches.push(quote! {
Expand Down Expand Up @@ -238,8 +238,8 @@ pub fn manager_impl_library_configs(_attr: TokenStream, input: TokenStream) -> T
})
});

// Add per_validate_config match
per_validate_matches.push(quote! {
// Add pre_validate_config match
pre_validate_matches.push(quote! {
#enum_ident::#variant_ident(config) => {
config.pre_validate(api)?;
Ok(())
Expand Down Expand Up @@ -310,9 +310,9 @@ pub fn manager_impl_library_configs(_attr: TokenStream, input: TokenStream) -> T
.map_err(LibraryError::SerdeJsonError)
}

pub fn per_validate_config(&self, api: &dyn cosmwasm_std::Api) -> LibraryResult<()> {
pub fn pre_validate_config(&self, api: &dyn cosmwasm_std::Api) -> LibraryResult<()> {
match self {
#(#per_validate_matches,)*
#(#pre_validate_matches,)*
}
}

Expand Down
28 changes: 15 additions & 13 deletions program-manager/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,21 @@ valence-processor-utils = { workspace = true }
valence-program-registry-utils = { workspace = true }
valence-library-base = { workspace = true }

valence-splitter-library = { workspace = true }
valence-reverse-splitter-library = { workspace = true }
valence-astroport-lper = { workspace = true }
valence-astroport-withdrawer = { workspace = true }
valence-account-utils = { workspace = true }
valence-authorization = { workspace = true }
valence-processor = { workspace = true }
valence-program-registry = { workspace = true }
valence-forwarder-library = { workspace = true }
valence-osmosis-gamm-lper = { workspace = true }
valence-osmosis-gamm-withdrawer = { workspace = true }
valence-osmosis-cl-lper = { workspace = true }
valence-osmosis-cl-withdrawer = { workspace = true }
valence-splitter-library = { workspace = true }
valence-reverse-splitter-library = { workspace = true }
valence-astroport-lper = { workspace = true }
valence-astroport-withdrawer = { workspace = true }
valence-account-utils = { workspace = true }
valence-authorization = { workspace = true }
valence-processor = { workspace = true }
valence-program-registry = { workspace = true }
valence-forwarder-library = { workspace = true }
valence-osmosis-gamm-lper = { workspace = true }
valence-osmosis-gamm-withdrawer = { workspace = true }
valence-osmosis-cl-lper = { workspace = true }
valence-osmosis-cl-withdrawer = { workspace = true }
valence-generic-ibc-transfer-library = { workspace = true }
valence-neutron-ibc-transfer-library = { workspace = true }

tokio = { workspace = true }
aho-corasick = "1.1"
Expand Down
167 changes: 160 additions & 7 deletions program-manager/schema/valence-program-manager.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,28 @@
},
"additionalProperties": false
},
"IbcTransferAmount": {
"oneOf": [
{
"type": "string",
"enum": [
"full_amount"
]
},
{
"type": "object",
"required": [
"fixed_amount"
],
"properties": {
"fixed_amount": {
"$ref": "#/definitions/Uint128"
}
},
"additionalProperties": false
}
]
},
"Int64": {
"description": "An implementation of i64 that is using strings for JSON encoding/decoding, such that the full i64 range can be used for clients that convert JSON numbers to floats, like JavaScript and jq.\n\n# Examples\n\nUse `from` to create instances of this and `i64` to get the value out:\n\n``` # use cosmwasm_std::Int64; let a = Int64::from(258i64); assert_eq!(a.i64(), 258); ```",
"type": "string"
Expand Down Expand Up @@ -752,14 +774,38 @@
},
"additionalProperties": false
},
{
"type": "object",
"required": [
"ValenceGenericIbcTransferLibrary"
],
"properties": {
"ValenceGenericIbcTransferLibrary": {
"$ref": "#/definitions/LibraryConfigUpdate9"
}
},
"additionalProperties": false
},
{
"type": "object",
"required": [
"ValenceNeutronIbcTransferLibrary"
],
"properties": {
"ValenceNeutronIbcTransferLibrary": {
"$ref": "#/definitions/LibraryConfigUpdate9"
}
},
"additionalProperties": false
},
{
"type": "object",
"required": [
"ValenceOsmosisClLper"
],
"properties": {
"ValenceOsmosisClLper": {
"$ref": "#/definitions/LibraryConfigUpdate9"
"$ref": "#/definitions/LibraryConfigUpdate10"
}
},
"additionalProperties": false
Expand All @@ -771,14 +817,50 @@
],
"properties": {
"ValenceOsmosisClWithdrawer": {
"$ref": "#/definitions/LibraryConfigUpdate10"
"$ref": "#/definitions/LibraryConfigUpdate11"
}
},
"additionalProperties": false
}
]
},
"LibraryConfigUpdate10": {
"type": "object",
"properties": {
"input_addr": {
"anyOf": [
{
"$ref": "#/definitions/LibraryAccountType"
},
{
"type": "null"
}
]
},
"lp_config": {
"anyOf": [
{
"$ref": "#/definitions/LiquidityProviderConfig3"
},
{
"type": "null"
}
]
},
"output_addr": {
"anyOf": [
{
"$ref": "#/definitions/LibraryAccountType"
},
{
"type": "null"
}
]
}
},
"additionalProperties": false
},
"LibraryConfigUpdate11": {
"type": "object",
"properties": {
"input_addr": {
Expand Down Expand Up @@ -1078,27 +1160,34 @@
"LibraryConfigUpdate9": {
"type": "object",
"properties": {
"input_addr": {
"amount": {
"anyOf": [
{
"$ref": "#/definitions/LibraryAccountType"
"$ref": "#/definitions/IbcTransferAmount"
},
{
"type": "null"
}
]
},
"lp_config": {
"denom": {
"anyOf": [
{
"$ref": "#/definitions/LiquidityProviderConfig3"
"$ref": "#/definitions/UncheckedDenom"
},
{
"type": "null"
}
]
},
"output_addr": {
"denom_to_pfm_map": {
"type": [
"object",
"null"
],
"additionalProperties": false
},
"input_addr": {
"anyOf": [
{
"$ref": "#/definitions/LibraryAccountType"
Expand All @@ -1107,6 +1196,28 @@
"type": "null"
}
]
},
"memo": {
"type": [
"string",
"null"
]
},
"output_addr": {
"type": [
"string",
"null"
]
},
"remote_chain_info": {
"anyOf": [
{
"$ref": "#/definitions/RemoteChainInfo"
},
{
"type": "null"
}
]
}
},
"additionalProperties": false
Expand Down Expand Up @@ -1381,6 +1492,26 @@
},
"additionalProperties": false
},
"PacketForwardMiddlewareConfig": {
"type": "object",
"required": [
"hop_chain_receiver_address",
"hop_to_destination_chain_channel_id",
"local_to_hop_chain_channel_id"
],
"properties": {
"hop_chain_receiver_address": {
"type": "string"
},
"hop_to_destination_chain_channel_id": {
"type": "string"
},
"local_to_hop_chain_channel_id": {
"type": "string"
}
},
"additionalProperties": false
},
"PairType": {
"oneOf": [
{
Expand Down Expand Up @@ -1743,6 +1874,28 @@
},
"additionalProperties": false
},
"RemoteChainInfo": {
"type": "object",
"required": [
"channel_id"
],
"properties": {
"channel_id": {
"type": "string"
},
"ibc_transfer_timeout": {
"anyOf": [
{
"$ref": "#/definitions/Uint64"
},
{
"type": "null"
}
]
}
},
"additionalProperties": false
},
"RetryLogic": {
"type": "object",
"required": [
Expand Down
9 changes: 8 additions & 1 deletion program-manager/src/domain/cosmos_cw.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use cosmos_grpc_client::{
cosmrs::bip32::secp256k1::sha2::{digest::Update, Digest, Sha256, Sha512},
BroadcastMode, Decimal, GrpcClient, ProstMsgNameToAny, Wallet,
};
use cosmwasm_std::{from_json, instantiate2_address, to_json_binary};
use cosmwasm_std::{from_json, instantiate2_address, testing::MockApi, to_json_binary};
use futures::future::BoxFuture;
use serde_json::to_vec;
use strum::VariantNames;
Expand Down Expand Up @@ -87,6 +87,7 @@ pub struct CosmosCosmwasmConnector {
code_ids: HashMap<String, u64>,
chain_name: String,
prefix: String,
api: MockApi,
}

impl fmt::Debug for CosmosCosmwasmConnector {
Expand Down Expand Up @@ -133,6 +134,8 @@ impl CosmosCosmwasmConnector {
code_ids: code_ids.clone(),
chain_name: chain_info.name.clone(),
prefix: chain_info.prefix.clone(),
api: cosmwasm_std::testing::MockApi::default()
.with_prefix(Box::leak(Box::new(chain_info.prefix.clone()))),
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't think of a way where this can go wrong but it'd be nice to add some comments about why Box::leak is fine here. Guess we could also have a chain prefix enum (with to/from string impl) which would make it static so we wouldn't need to deal with boxing like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @bekauz and @stiiifff
We have a weird play with the connectors, because they are not cached for the entire program, because the manager is not a program, rather a library, so they are cached for the functions (init , update, migrate).
This was leaking the prefix on every time we call a function, not much, and can easily fixed with a server restart, but still, it is a leak.

I just copied the MockApi from cosmwas_std, and changed the api to accept String instead of the static str, which doesn't leak memory now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also allowed us to remove our helpers for it and just use the mock api.

})
}
}
Expand Down Expand Up @@ -878,6 +881,10 @@ impl Connector for CosmosCosmwasmConnector {
Ok(from_json::<ProgramConfig>(&res.program_config)
.map_err(CosmosCosmwasmError::CosmwasmStdError)?)
}

fn get_api(&self) -> &MockApi {
&self.api
}
}

// Helpers
Expand Down
2 changes: 2 additions & 0 deletions program-manager/src/domain/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use async_trait::async_trait;
use cosmos_cw::{CosmosCosmwasmConnector, CosmosCosmwasmError};

use cosmwasm_schema::schemars::JsonSchema;
use cosmwasm_std::testing::MockApi;
use serde::{Deserialize, Serialize};

// use cosmos_evm::CosmosEvmError;
Expand Down Expand Up @@ -174,6 +175,7 @@ pub trait Connector: fmt::Debug + Send + Sync {
// Verify the bridge account was instantiated
async fn verify_bridge_account(&mut self, bridge_addr: String) -> ConnectorResult<()>;

fn get_api(&self) -> &MockApi;
// ---------------------------------------------------------------------------------------
// Below are functions that sohuld only be implemented on a specific domain
// For example authorization contract methods should only be implemented on the main domain
Expand Down
Loading
Loading