Skip to content

Commit

Permalink
Make setting initial payees part of proposeConfig
Browse files Browse the repository at this point in the history
  • Loading branch information
@archseer
archseer committed Feb 10, 2022
1 parent 06317d2 commit 43a5496
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 102 deletions.
164 changes: 98 additions & 66 deletions contracts/ocr2/src/contract.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ pub fn execute(
id,
signers,
transmitters,
payees,
f,
onchain_config,
} => {
Expand All @@ -127,13 +128,18 @@ pub fn execute(
.iter()
.map(|t| api.addr_validate(t))
.collect::<StdResult<Vec<Addr>>>()?;
let payees = payees
.iter()
.map(|t| api.addr_validate(t))
.collect::<StdResult<Vec<Addr>>>()?;
execute_propose_config(
deps,
env,
info,
id,
signers,
transmitters,
payees,
f,
onchain_config.0,
)
Expand Down Expand Up @@ -190,7 +196,6 @@ pub fn execute(
ExecuteMsg::WithdrawFunds { recipient, amount } => {
execute_withdraw_funds(deps, env, info, recipient, amount)
}
ExecuteMsg::SetPayees { payees } => execute_set_payees(deps, env, info, payees),
ExecuteMsg::TransferPayeeship {
transmitter,
proposed,
Expand Down Expand Up @@ -379,9 +384,16 @@ pub fn execute_accept_proposal(
for key in keys {
SIGNERS.remove(deps.storage, &key);
}
// NOTE: we don't clear payees to reduce gas
// let keys: Vec<_> = PAYEES
// .keys(deps.storage, None, None, Order::Ascending)
// .collect();
// for key in keys {
// PAYEES.remove(deps.storage, &key);
// }

// Update oracle set
for (signer, transmitter) in &proposal.oracles {
for (signer, transmitter, payee) in &proposal.oracles {
SIGNERS.update(deps.storage, signer, |value| {
require!(value.is_none(), RepeatedAddress);
Ok(())
Expand All @@ -394,6 +406,8 @@ pub fn execute_accept_proposal(
from_round_id: config.latest_aggregator_round_id,
})
})?;

PAYEES.save(deps.storage, &transmitter, &payee)?;
}

// calculate onchain_config from stored config
Expand All @@ -402,6 +416,12 @@ pub fn execute_accept_proposal(
onchain_config.extend_from_slice(&config.min_answer.to_be_bytes());
onchain_config.extend_from_slice(&config.max_answer.to_be_bytes());

let oracles: Vec<_> = proposal
.oracles
.iter()
.map(|(signer, transmitter, _payee)| (signer, transmitter))
.collect();

// Update config
let (previous_config_block_number, config) = {
config.f = proposal.f;
Expand All @@ -412,7 +432,7 @@ pub fn execute_accept_proposal(
&env.block.chain_id,
&env.contract.address,
config.config_count,
&proposal.oracles,
&oracles,
proposal.f,
&onchain_config,
proposal.offchain_config_version,
Expand All @@ -429,12 +449,12 @@ pub fn execute_accept_proposal(
let signers = proposal
.oracles
.iter()
.map(|(signer, _)| attr("signers", hex::encode(&signer.0)));
.map(|(signer, _, _)| attr("signers", hex::encode(&signer.0)));

let transmitters = proposal
.oracles
.iter()
.map(|(_, transmitter)| attr("transmitters", transmitter));
.map(|(_, transmitter, _)| attr("transmitters", transmitter));

response = response.add_event(
Event::new("propose_config")
Expand Down Expand Up @@ -467,13 +487,52 @@ pub fn execute_accept_proposal(
Ok(response)
}

// Can't be used to change payee addresses, only to initially populate them.
pub fn execute_propose_payees(
deps: DepsMut,
_env: Env,
info: MessageInfo,
payees: Vec<(String, String)>, // (transmitter, payee)
) -> Result<Response, ContractError> {
require!(OWNER.is_owner(deps.as_ref(), &info.sender)?, Unauthorized);

let mut events = Vec::with_capacity(payees.len());

let payees = payees
.iter()
.map(|(transmitter, payee)| -> StdResult<(Addr, Addr)> {
Ok((
deps.api.addr_validate(transmitter)?,
deps.api.addr_validate(payee)?,
))
})
.collect::<StdResult<Vec<_>>>()?;

for (transmitter, payee) in payees {
// Set the payee unless it's already set
PAYEES.update(deps.storage, &transmitter, |value| {
if value.is_some() {
return Err(ContractError::PayeeAlreadySet);
}
events.push(
Event::new("payeeship_transferred")
.add_attribute("transmitter", &transmitter)
.add_attribute("current", &payee),
);
Ok(payee)
})?;
}
Ok(Response::default().add_events(events))
}

pub fn execute_propose_config(
deps: DepsMut,
_env: Env,
info: MessageInfo,
id: ProposalId,
signers: Vec<Binary>,
transmitters: Vec<Addr>,
payees: Vec<Addr>,
f: u8,
onchain_config: Vec<u8>,
) -> Result<Response, ContractError> {
Expand All @@ -498,7 +557,12 @@ pub fn execute_propose_config(
require!(!proposal.finalized, InvalidInput);

proposal.f = f;
proposal.oracles = signers.into_iter().zip(transmitters.into_iter()).collect();
proposal.oracles = signers
.into_iter()
.zip(transmitters.into_iter())
.zip(payees.into_iter())
.map(|((signers, transmitters), payees)| (signers, transmitters, payees))
.collect();

PROPOSALS.save(deps.storage, id.u128().into(), &proposal)?;

Expand Down Expand Up @@ -1428,44 +1492,6 @@ fn calculate_reimbursement(
// --- Payee management
// ---

// Can't be used to change payee addresses, only to initially populate them.
pub fn execute_set_payees(
deps: DepsMut,
_env: Env,
info: MessageInfo,
payees: Vec<(String, String)>, // (transmitter, payee)
) -> Result<Response, ContractError> {
require!(OWNER.is_owner(deps.as_ref(), &info.sender)?, Unauthorized);

let mut events = Vec::with_capacity(payees.len());

let payees = payees
.iter()
.map(|(transmitter, payee)| -> StdResult<(Addr, Addr)> {
Ok((
deps.api.addr_validate(transmitter)?,
deps.api.addr_validate(payee)?,
))
})
.collect::<StdResult<Vec<_>>>()?;

for (transmitter, payee) in payees {
// Set the payee unless it's already set
PAYEES.update(deps.storage, &transmitter, |value| {
if value.is_some() {
return Err(ContractError::PayeeAlreadySet);
}
events.push(
Event::new("payeeship_transferred")
.add_attribute("transmitter", &transmitter)
.add_attribute("current", &payee),
);
Ok(payee)
})?;
}
Ok(Response::default().add_events(events))
}

pub fn execute_transfer_payeeship(
deps: DepsMut,
_env: Env,
Expand Down Expand Up @@ -1596,6 +1622,12 @@ pub(crate) mod tests {
"transmitter2".to_string(),
"transmitter3".to_string(),
],
payees: vec![
"transmitter0".to_string(),
"transmitter1".to_string(),
"transmitter2".to_string(),
"transmitter3".to_string(),
],
f: 1,
onchain_config: Binary(vec![]),
};
Expand Down Expand Up @@ -1655,28 +1687,28 @@ pub(crate) mod tests {
#[test]
fn payees() {
let mut deps = setup();
let owner = "owner".to_string();

let msg = ExecuteMsg::SetPayees {
payees: vec![("transmitter0".to_string(), "payee0".to_string())],
};
let execute_info = mock_info(&owner, &[]);
execute(deps.as_mut(), mock_env(), execute_info, msg).unwrap();

// setting the same payee again fails
let msg = ExecuteMsg::SetPayees {
payees: vec![("transmitter0".to_string(), "payee1".to_string())],
};
let execute_info = mock_info(&owner, &[]);
let res = execute(deps.as_mut(), mock_env(), execute_info, msg);
assert_eq!(res.unwrap_err(), ContractError::PayeeAlreadySet);

// setting a different payee works
let msg = ExecuteMsg::SetPayees {
payees: vec![("transmitter1".to_string(), "payee1".to_string())],
};
let execute_info = mock_info(&owner, &[]);
execute(deps.as_mut(), mock_env(), execute_info, msg).unwrap();
// let owner = "owner".to_string();

// let msg = ExecuteMsg::ProposePayees {
// payees: vec![("transmitter0".to_string(), "payee0".to_string())],
// };
// let execute_info = mock_info(&owner, &[]);
// execute(deps.as_mut(), mock_env(), execute_info, msg).unwrap();

// // setting the same payee again fails
// let msg = ExecuteMsg::ProposePayees {
// payees: vec![("transmitter0".to_string(), "payee1".to_string())],
// };
// let execute_info = mock_info(&owner, &[]);
// let res = execute(deps.as_mut(), mock_env(), execute_info, msg);
// assert_eq!(res.unwrap_err(), ContractError::PayeeAlreadySet);

// // setting a different payee works
// let msg = ExecuteMsg::ProposePayees {
// payees: vec![("transmitter1".to_string(), "payee1".to_string())],
// };
// let execute_info = mock_info(&owner, &[]);
// execute(deps.as_mut(), mock_env(), execute_info, msg).unwrap();

// can't transfer to self
let msg = ExecuteMsg::TransferPayeeship {
Expand Down
39 changes: 11 additions & 28 deletions contracts/ocr2/src/integration_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,11 @@ fn setup() -> Env {
id,
signers,
transmitters: transmitters.clone(),
payees: transmitters
.iter()
.enumerate()
.map(|(i, _)| format!("payee{}", i))
.collect(),
f: 1,
onchain_config: Binary(vec![]),
};
Expand Down Expand Up @@ -409,21 +414,6 @@ fn transmit_happy_path() {
.unwrap();
assert_eq!(owed_payment, observation_payment + reimbursement);

// set_payees so we can withdraw
let msg = ExecuteMsg::SetPayees {
payees: env
.transmitters
.iter()
.enumerate()
.map(|(i, transmitter)| (transmitter.clone(), format!("payee{}", i)))
.collect(),
};
env.router
.execute_contract(env.owner.clone(), env.ocr2_addr.clone(), &msg, &[])
.unwrap();

// TODO: what happens if an oracle has no payees attached?
// https://github.com/smartcontractkit/chainlink-terra/issues/20
let available: LinkAvailableForPaymentResponse = env
.router
.wrap()
Expand Down Expand Up @@ -534,6 +524,12 @@ fn transmit_happy_path() {
id,
signers,
transmitters: env.transmitters.clone(),
payees: env
.transmitters
.iter()
.enumerate()
.map(|(i, _)| format!("payee{}", i))
.collect(),
f: 5,
onchain_config: Binary(vec![]),
};
Expand Down Expand Up @@ -723,19 +719,6 @@ fn set_link_token() {
// 1 round + gas reimbursement
assert_eq!(owed_payment, observation_payment + reimbursement);

// set_payees so we can withdraw
let msg = ExecuteMsg::SetPayees {
payees: env
.transmitters
.iter()
.enumerate()
.map(|(i, transmitter)| (transmitter.clone(), format!("payee{}", i)))
.collect(),
};
env.router
.execute_contract(env.owner.clone(), env.ocr2_addr.clone(), &msg, &[])
.unwrap();

// ^ ---- all duplicated from transmit_happy_path()

let new_link_token = env
Expand Down
4 changes: 1 addition & 3 deletions contracts/ocr2/src/msg.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ pub enum ExecuteMsg {
id: ProposalId,
signers: Vec<Binary>,
transmitters: Vec<String>,
payees: Vec<String>,
f: u8,
onchain_config: Binary,
},
Expand Down Expand Up @@ -98,9 +99,6 @@ pub enum ExecuteMsg {
/// Handler for LINK token Receive message
Receive(Cw20ReceiveMsg),

SetPayees {
payees: Vec<(String, String)>,
},
TransferPayeeship {
transmitter: String,
proposed: String,
Expand Down
9 changes: 4 additions & 5 deletions contracts/ocr2/src/state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub struct Config {
pub struct Proposal {
pub owner: Addr,
pub finalized: bool,
pub oracles: Vec<(Binary, Addr)>,
pub oracles: Vec<(Binary, Addr, Addr)>, // (signer, transmitter, payee)
pub f: u8,
pub offchain_config_version: u64,
pub offchain_config: Binary,
Expand All @@ -102,11 +102,10 @@ impl Proposal {
use blake2::{Blake2s, Digest};
let mut hasher = Blake2s::default();
hasher.update([(self.oracles.len() as u8)]);
for (signer, _) in &self.oracles {
for (signer, transmitter, payee) in &self.oracles {
hasher.update(&signer.0);
}
for (_, transmitter) in &self.oracles {
hasher.update(transmitter.as_bytes());
hasher.update(payee.as_bytes());
}
hasher.update(&[self.f]);
hasher.update(&self.offchain_config_version.to_be_bytes());
Expand All @@ -121,7 +120,7 @@ pub fn config_digest_from_data(
chain_id: &str,
contract_address: &Addr,
config_count: u32,
oracles: &[(Binary, Addr)],
oracles: &[(&Binary, &Addr)],
f: u8,
onchain_config: &[u8],
offchain_config_version: u64,
Expand Down

0 comments on commit 43a5496

Please sign in to comment.